[rfc][icedtea-web] exception flaw in usages of PluginDebug.debug(...)

Deepak Bhole dbhole at redhat.com
Tue Aug 7 12:37:00 PDT 2012


* Jiri Vanek <jvanek at redhat.com> [2012-08-07 11:47]:
> Hi! Today Jana during testing of js api of IcedTEa-Web have
> encountered exception which was caused by PluginDebug.Debug. Even
> when DEBUG was disabled, then it caused NPE or ArrayOutOfBounds
> exceptions
> 
> eg PluginDebug.debug(object.something()) when object was null, then
> icedtea-web have fail even when debug was off.
> 
> This patch is fixing it.
> 
> Not all occurrences of PluginDebug.debug() were "fixed",  but just those affected by issue.
> 
> I have also fortified debug() method itself a bit.
> 
> Tahanx for comments
> 
> J.

Hi Jiri,

A lot of new lines are missing a space before the "{" ... per coding
style requirement, there should be a space so please add them -- or
better yet, use the Eclipse project file in the repo :)

Also, why was the DEBUG variable made public? Does it need to be
mutable?

Cheers,
Deepak

> diff -r 31b729370710 plugin/icedteanp/java/netscape/javascript/JSRunnable.java
> --- a/plugin/icedteanp/java/netscape/javascript/JSRunnable.java	Tue Aug 07 12:24:29 2012 +0200
> +++ b/plugin/icedteanp/java/netscape/javascript/JSRunnable.java	Tue Aug 07 17:21:17 2012 +0200
> @@ -65,7 +65,9 @@
>                  notifyAll();
>              }
>          } catch (Throwable t) {
> -            PluginDebug.debug(t.toString());
> +            if (PluginDebug.DEBUG){
> +                PluginDebug.debug(t.toString());
> +            }
>              t.printStackTrace(System.err);
>          }
>      }
> diff -r 31b729370710 plugin/icedteanp/java/sun/applet/AppletSecurityContextManager.java
> --- a/plugin/icedteanp/java/sun/applet/AppletSecurityContextManager.java	Tue Aug 07 12:24:29 2012 +0200
> +++ b/plugin/icedteanp/java/sun/applet/AppletSecurityContextManager.java	Tue Aug 07 17:21:17 2012 +0200
> @@ -61,7 +61,9 @@
>      }
>  
>      public static void handleMessage(int identifier, int reference, String src, String[] privileges, String message) {
> -        PluginDebug.debug(identifier, " -- ", src, " -- ", reference, " -- ", message, " CONTEXT= ", contexts.get(identifier));
> +        if (PluginDebug.DEBUG){
> +            PluginDebug.debug(identifier, " -- ", src, " -- ", reference, " -- ", message, " CONTEXT= ", contexts.get(identifier));
> +        }
>          AccessControlContext callContext = null;
>  
>          privileges = privileges != null ? privileges : new String[0];
> diff -r 31b729370710 plugin/icedteanp/java/sun/applet/PluginAppletSecurityContext.java
> --- a/plugin/icedteanp/java/sun/applet/PluginAppletSecurityContext.java	Tue Aug 07 12:24:29 2012 +0200
> +++ b/plugin/icedteanp/java/sun/applet/PluginAppletSecurityContext.java	Tue Aug 07 17:21:17 2012 +0200
> @@ -382,7 +382,9 @@
>                      o = m = c.getMethod(methodName, signature.getClassArray());
>                      store.reference(m);
>                  }
> -                PluginDebug.debug(o, " has id ", store.getIdentifier(o));
> +                if (PluginDebug.DEBUG){
> +                    PluginDebug.debug(o, " has id ", store.getIdentifier(o));
> +                }
>                  write(reference, args[0] + " " + store.getIdentifier(o));
>              } else if (message.startsWith("GetStaticFieldID")
>                                          || message.startsWith("GetFieldID")) {
> @@ -392,9 +394,9 @@
>                  String fieldName = (String) store.getObject(fieldID);
>  
>                  Class<?> c = (Class<?>) store.getObject(classID);
> -
> -                PluginDebug.debug("GetStaticFieldID/GetFieldID got class=", c.getName());
> -
> +                if (PluginDebug.DEBUG){
> +                  PluginDebug.debug("GetStaticFieldID/GetFieldID got class=", c.getName());
> +                }
>                  Field f = null;
>                  f = c.getField(fieldName);
>  
> @@ -632,7 +634,9 @@
>                  arguments[1] = methodName;
>                  for (int i = 0; i < args.length - 3; i++) {
>                      arguments[i + 2] = store.getObject(parseCall(args[3 + i], null, Integer.class));
> -                    PluginDebug.debug("GOT ARG: ", arguments[i + 2]);
> +                    if (PluginDebug.DEBUG){
> +                        PluginDebug.debug("GOT ARG: ", arguments[i + 2]);
> +                    }
>                  }
>  
>                  Object[] matchingMethodAndArgs = MethodOverloadResolver.getMatchingMethod(arguments);
> @@ -794,12 +798,9 @@
>                  b = o.getBytes("UTF-16LE");
>                  buf = new StringBuffer(b.length * 2);
>                  buf.append(b.length);
> -                for (int i = 0; i < b.length; i++)
> -                    buf
> -                                                        .append(" "
> -                                                                        + Integer
> -                                                                                        .toString(((int) b[i]) & 0x0ff, 16));
> -
> +                for (int i = 0; i < b.length; i++) {
> +                    buf.append(" " + Integer.toString(((int) b[i]) & 0x0ff, 16));
> +                }
>                  PluginDebug.debug("Java: GetStringChars: ", o);
>                  PluginDebug.debug("  String BYTES: ", buf);
>                  write(reference, "GetStringChars " + buf);
> @@ -814,12 +815,9 @@
>                  b = o.getBytes("UTF-8");
>                  buf = new StringBuffer(b.length * 2);
>                  buf.append(b.length);
> -                for (int i = 0; i < b.length; i++)
> -                    buf
> -                            .append(" "
> -                                    + Integer
> -                                            .toString(((int) b[i]) & 0x0ff, 16));
> -
> +                for (int i = 0; i < b.length; i++) {
> +                    buf.append(" " + Integer.toString(((int) b[i]) & 0x0ff, 16));
> +                }
>                  write(reference, "GetToStringValue " + buf);
>              } else if (message.startsWith("NewArray")) {
>                  String[] args = message.split(" ");
> @@ -966,7 +964,9 @@
>                  for (int i = 0; i < args.length - 2; i++) {
>                      arguments[i + 1] = store.getObject(parseCall(args[2 + i],
>                              null, Integer.class));
> -                    PluginDebug.debug("GOT ARG: ", arguments[i + 1]);
> +                    if (PluginDebug.DEBUG){
> +                        PluginDebug.debug("GOT ARG: ", arguments[i + 1]);
> +                    }
>                  }
>  
>                  Object[] matchingConstructorAndArgs = MethodOverloadResolver
> diff -r 31b729370710 plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
> --- a/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java	Tue Aug 07 12:24:29 2012 +0200
> +++ b/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java	Tue Aug 07 17:21:17 2012 +0200
> @@ -131,7 +131,9 @@
>              public NetxPanel run() {
>                  NetxPanel panel = new NetxPanel(doc, atts, false);
>                  NetxPanel.debug("Using NetX panel");
> -                PluginDebug.debug(atts.toString());
> +                if (PluginDebug.DEBUG){
> +                    PluginDebug.debug(atts.toString());
> +                }
>                  return panel;
>              }
>          });
> @@ -170,8 +172,9 @@
>              streamhandler.write("instance " + identifier + " reference " + -1 + " fatalError: " + "Initialization timed out");
>              return null;
>          }
> -
> -        PluginDebug.debug("Applet ", a.getClass(), " initialized");
> +        if (PluginDebug.DEBUG){
> +            PluginDebug.debug("Applet ", a.getClass(), " initialized");
> +        }
>          streamhandler.write("instance " + identifier + " reference 0 initialized");
>  
>          /* AppletViewerPanel sometimes doesn't set size right initially. This 
> @@ -765,8 +768,9 @@
>              // Wait for the panel to initialize
>              // (happens in a separate thread)
>              waitForAppletInit(panel);
> -
> -            PluginDebug.debug(panel, " -- ", panel.getApplet(), " -- ", panel.isAlive());
> +            if (PluginDebug.DEBUG){
> +                PluginDebug.debug(panel, " -- ", panel.getApplet(), " -- ", panel.isAlive());
> +            }
>  
>              // Still null?
>              if (panel.getApplet() == null) {
> @@ -777,8 +781,10 @@
>              o = panel.getApplet();
>              PluginDebug.debug("Looking for object ", o, " panel is ", panel);
>              AppletSecurityContextManager.getSecurityContext(0).store(o);
> -            PluginDebug.debug("WRITING 1: ", "context 0 reference ", reference, " GetJavaObject "
> +            if (PluginDebug.DEBUG){
> +                PluginDebug.debug("WRITING 1: ", "context 0 reference ", reference, " GetJavaObject "
>                                   , AppletSecurityContextManager.getSecurityContext(0).getIdentifier(o));
> +            }
>              streamhandler.write("context 0 reference " + reference + " GetJavaObject "
>                                + AppletSecurityContextManager.getSecurityContext(0).getIdentifier(o));
>              PluginDebug.debug("WRITING 1 DONE");
> @@ -1026,7 +1032,9 @@
>                  PluginDebug.debug("wait getMEM request 2");
>                  while (request.isDone() == false)
>                      request.wait();
> -                PluginDebug.debug("wait getMEM request 3 GOT: ", request.getObject().getClass());
> +                if (PluginDebug.DEBUG){
> +                    PluginDebug.debug("wait getMEM request 3 GOT: ", request.getObject().getClass());
> +                }
>              }
>          } catch (InterruptedException e) {
>              throw new RuntimeException("Interrupted waiting for call request.",
> @@ -1078,7 +1086,9 @@
>          streamhandler.postCallRequest(request);
>          streamhandler.write(request.getMessage());
>          try {
> -            PluginDebug.debug("wait setMem request: ", request.getMessage());
> +            if (PluginDebug.DEBUG){
> +                PluginDebug.debug("wait setMem request: ", request.getMessage());
> +            }
>              PluginDebug.debug("wait setMem request 1");
>              synchronized (request) {
>                  PluginDebug.debug("wait setMem request 2");
> @@ -1319,7 +1329,9 @@
>  
>              requestURI = UrlUtil.encode(scheme + "://" + uri.getHost() + port + "/" + uri.getPath(), "UTF-8");
>          } catch (Exception e) {
> -            PluginDebug.debug("Cannot construct URL from ", uri.toString(), " ... falling back to DIRECT proxy");
> +            if (PluginDebug.DEBUG){
> +                PluginDebug.debug("Cannot construct URL from ", uri.toString(), " ... falling back to DIRECT proxy");
> +            }
>              e.printStackTrace();
>              return null;
>          }
> @@ -1723,8 +1735,9 @@
>                      // -- encountered ... is > next?
>                      if ((c[0] = in.read()) == '>') {
>                          buf.append((char) c[0]);
> -
> -                        PluginDebug.debug("Comment skipped: ", buf.toString());
> +                        if (PluginDebug.DEBUG){
> +                            PluginDebug.debug("Comment skipped: ", buf.toString());
> +                        }
>  
>                          // comment skipped.
>                          return;
> @@ -1736,7 +1749,9 @@
>  
>              } else if (commentHeaderPassed == false) {
>                  buf.append((char) c[0]);
> -                PluginDebug.debug("Warning: Attempted to skip comment, but this tag does not appear to be a comment: ", buf.toString());
> +                if (PluginDebug.DEBUG){
> +                    PluginDebug.debug("Warning: Attempted to skip comment, but this tag does not appear to be a comment: ", buf.toString());
> +                }
>                  return;
>              }
>  
> diff -r 31b729370710 plugin/icedteanp/java/sun/applet/PluginDebug.java
> --- a/plugin/icedteanp/java/sun/applet/PluginDebug.java	Tue Aug 07 12:24:29 2012 +0200
> +++ b/plugin/icedteanp/java/sun/applet/PluginDebug.java	Tue Aug 07 17:21:17 2012 +0200
> @@ -39,18 +39,25 @@
>  
>  public class PluginDebug {
>  
> -    static final boolean DEBUG = System.getenv().containsKey("ICEDTEAPLUGIN_DEBUG");
> +    public static final boolean DEBUG = System.getenv().containsKey("ICEDTEAPLUGIN_DEBUG");
>  
>      public static void debug(Object... messageChunks) {
> -        if (DEBUG) {
> +        if (DEBUG) try{
>              if (messageChunks == null) {
>                  messageChunks = new Object[] {null};
>              }
>              StringBuilder b = new StringBuilder();
>              for (Object chunk : messageChunks) {
> +                if (chunk==null){
> +                    chunk="null";
> +                }
>                  b.append(chunk);
>              }
>              System.err.println(b.toString());
> +        }catch(Exception ex){
> +            //lets debug will not kill applet
> +            System.err.println("Error in debug mechanism:");
> +            ex.printStackTrace();
>          }
>      }
>  }
> diff -r 31b729370710 plugin/icedteanp/java/sun/applet/PluginMessageConsumer.java
> --- a/plugin/icedteanp/java/sun/applet/PluginMessageConsumer.java	Tue Aug 07 12:24:29 2012 +0200
> +++ b/plugin/icedteanp/java/sun/applet/PluginMessageConsumer.java	Tue Aug 07 17:21:17 2012 +0200
> @@ -206,7 +206,9 @@
>  
>          for (PluginMessageHandlerWorker worker : workers) {
>              if (worker.isFree(prioritized)) {
> -                PluginDebug.debug("Found free worker (", worker.isPriority(), ") with id ", worker.getWorkerId());
> +                if (PluginDebug.DEBUG){
> +                    PluginDebug.debug("Found free worker (", worker.isPriority(), ") with id ", worker.getWorkerId());
> +                }
>                  // mark it busy before returning
>                  worker.busy();
>                  return worker;
> @@ -218,10 +220,14 @@
>              PluginMessageHandlerWorker worker = null;
>  
>              if (workers.size() < (MAX_WORKERS - PRIORITY_WORKERS)) {
> -                PluginDebug.debug("Cannot find free worker, creating worker ", workers.size());
> +                if (PluginDebug.DEBUG){
> +                    PluginDebug.debug("Cannot find free worker, creating worker ", workers.size());
> +                }
>                  worker = new PluginMessageHandlerWorker(this, streamHandler, workers.size(), false);
>              } else if (prioritized) {
> -                PluginDebug.debug("Cannot find free worker, creating priority worker ", workers.size());
> +                if (PluginDebug.DEBUG){
> +                    PluginDebug.debug("Cannot find free worker, creating priority worker ", workers.size());
> +                }
>                  worker = new PluginMessageHandlerWorker(this, streamHandler, workers.size(), true);
>              } else {
>                  return null;
> diff -r 31b729370710 plugin/icedteanp/java/sun/applet/PluginProxySelector.java
> --- a/plugin/icedteanp/java/sun/applet/PluginProxySelector.java	Tue Aug 07 12:24:29 2012 +0200
> +++ b/plugin/icedteanp/java/sun/applet/PluginProxySelector.java	Tue Aug 07 17:21:17 2012 +0200
> @@ -106,8 +106,9 @@
>          }
>  
>          proxyList.add(proxy);
> -
> -        PluginDebug.debug("Proxy for ", uri.toString(), " is ", proxy);
> +        if (PluginDebug.DEBUG){
> +            PluginDebug.debug("Proxy for ", uri.toString(), " is ", proxy);
> +        }
>  
>          return proxyList;
>      }
> diff -r 31b729370710 plugin/icedteanp/java/sun/applet/PluginStreamHandler.java
> --- a/plugin/icedteanp/java/sun/applet/PluginStreamHandler.java	Tue Aug 07 12:24:29 2012 +0200
> +++ b/plugin/icedteanp/java/sun/applet/PluginStreamHandler.java	Tue Aug 07 17:21:17 2012 +0200
> @@ -146,8 +146,9 @@
>                  }
>              }
>          }
> -
> -        PluginDebug.debug("readPair: '", array[0], "' - '", array[1], "' ", end);
> +        if (PluginDebug.DEBUG){
> +            PluginDebug.debug("readPair: '", array[0], "' - '", array[1], "' ", end);
> +        }
>          return end;
>      }
>  
> diff -r 31b729370710 plugin/icedteanp/java/sun/applet/RequestQueue.java
> --- a/plugin/icedteanp/java/sun/applet/RequestQueue.java	Tue Aug 07 12:24:29 2012 +0200
> +++ b/plugin/icedteanp/java/sun/applet/RequestQueue.java	Tue Aug 07 17:21:17 2012 +0200
> @@ -43,7 +43,9 @@
>      private int size = 0;
>  
>      public void post(PluginCallRequest request) {
> -        PluginDebug.debug("Securitymanager=", System.getSecurityManager());
> +        if (PluginDebug.DEBUG){
> +            PluginDebug.debug("Securitymanager=", System.getSecurityManager());
> +        }
>          if (head == null) {
>              head = tail = request;
>              tail.setNext(null);




More information about the distro-pkg-dev mailing list