[rfc][icedtea-web] Remove need for HTML tag scanner in PluginAppletViewer
Pavel Tisnovsky
ptisnovs at redhat.com
Fri Nov 16 05:51:52 PST 2012
Hi Adam and Jiri,
I've looked mainly on C/C++ part of this patch and it seems ok from my side. I have
just a few comments:
escape_string(const char* to_encode):
* Escape characters for pssing to Java.
^^^ (I hope you mean to add 'a' here, not other vowel :D)
* this function is much more readable now thanks to using str:string :)
* "\n" for new line, "\\" for "\", "\:" for ";"
- so semicolon is escaped as \ + colon?
- I suppose it is done because you always add semicolon between the name and value
in parameter list, right?
- how about empty name and/or value?
- public static String decodeString(String toDecode) {
-
- toDecode = toDecode.replace(">", ">");
- toDecode = toDecode.replace("<", "<");
- toDecode = toDecode.replace("&", "&");
- toDecode = toDecode.replace(" ", "\n");
- toDecode = toDecode.replace(" ", "\r");
- toDecode = toDecode.replace(""", "\"");
-
- return toDecode;
- }
* How does it work with backslash? It should works, but Opera (for example)
has problems with this character sometime. Jiri - is this checked somewhere?
Cheers,
Pavel
----- Adam Domurad <adomurad at redhat.com> wrote:
> On 10/26/2012 09:13 AM, Jiri Vanek wrote:
> > On 10/25/2012 10:10 PM, Adam Domurad wrote:
> >> Hi all. This is something I've wanted to do for a while, simply
> >> because of the hackish nature of the
> >> applet/object/embed tag parsing code in ITW. Thoughts welcome, I'll
> >> still be doing some more testing
> >> on this (looks good so far) but would appreciate feedback. The patch
> >> also makes it possible to do
> >> unit tests from classes included in plugin.jar (used to unit test the
> >> new
> >> sun.java.PluginAppletAttributes).
> >>
> >> The applet tag information flows like this pre-patch:
> >> - NPAPI parses the tag information
> >> - ITW's C++ side takes in the already-parsed tag information, creates
> >> an embed tag (embed tag only)
> >> for ITW to use.
> >> - ITW's java side receives the tag, and scans it using (less than
> >> desirable) parsing routines.
> >>
> >> Post-patch:
> >> - NPAPI parses the tag information
> >> - ITW's C++ side generates a simple listing of the name value pairs
> >> passed
> >> - ITW's java side parses these name value pairs
> >
> > I like this patch. I found several issues which had come across my
> > mind, but I think during other reviews there will come more of them.
> > Also I'm not 100% capable to verify C code. SO before aprove I will
> > definitely check with Pavel or somebody else can do this. I checked c
> > part only from logical point of view.
> >>
> >> Points of contention:
> >> - PluginAppletViewer#parse had a 'ydisp' variable that has been
> >> changed to a static variable, since
> >> the parse method will now only ever handle one applet tag (the old
> >> version expected to have
> >> potentially multiple). However I'm not 100% about this because the
> >> old version as well only ever
> >> received one applet tag, rendering this effectively to always be 1...
> >> I'm not sure if the behaviour
> >> should be 'fixed' this way.
> >
> > Your arguments seemed valid. And although PluginAppletViewer is
> > singleton, making the ydisp variable static looks like it will behave
> > differently. I would recommend empiric testing :)) See how multiple
> > applets are shown on (multiple)page(s) and compare with your new
> > implementation :)
> >>
> >> - The code was made to behave as-it-were as much as possible, meaning
> >> it can print a warning about a
> >> "missing code attribute in the embed tag" no matter what tag was
> >> used. This is as it was because ITW
> >> would always get passed an embed tag. Feel free to force me to change
> >> it:)
> >>
> >> ChangeLog:
> >> 2012-10-25 Adam Domurad <adomurad at redhat.com>
> >>
> >> Remove the applet/embed/object tag parser from ITW. Send the applet
> >> parameters directly from the C++.
> >> * Makefile.am: Allow unit-testing for classes in plugin.jar.
> >> * plugin/icedteanp/IcedTeaNPPlugin.cc: Send quoted parameter
> >> name/values instead of applet tag. Remove some dead code.
> >> * plugin/icedteanp/IcedTeaNPPlugin.h: Rename applet_tag ->
> >> parameters_string.
> >> * plugin/icedteanp/java/sun/applet/PluginAppletViewer.java:
> >> Extract parsing code into its own class.
> >> * plugin/icedteanp/java/sun/applet/PluginAppletAttributes.java:
> >> New, encapsulates the (simplified) attribute parsing logic.
> >> * tests/netx/unit/sun/applet/PluginAppletAttributesTest.java:
> >> Unit tests for parsing logic.
> >>
> >>
> >> nuke-parser.patch
> >>
> >>
> >> diff --git a/Makefile.am b/Makefile.am
> >> --- a/Makefile.am
> >> +++ b/Makefile.am
> >> @@ -882,7 +882,7 @@ stamps/netx-unit-tests-compile.stamp: st
> >> mkdir -p $(NETX_UNIT_TEST_DIR) && \
> >> $(BOOT_DIR)/bin/javac $(IT_JAVACFLAGS) \
> >> -d $(NETX_UNIT_TEST_DIR) \
> >> - -classpath
> >> $(JUNIT_JAR):$(NETX_DIR)/lib/classes.jar:$(TEST_EXTENSIONS_DIR) \
> >> + -classpath
> >> $(JUNIT_JAR):$(DESTDIR)$(datadir)/$(PACKAGE_NAME)/plugin.jar:$(NETX_DIR)/lib/classes.jar:$(TEST_EXTENSIONS_DIR)
> >> \
> >> @netx-unit-tests-source-files.txt && \
> >> mkdir -p stamps && \
> >> touch $@
> >> @@ -912,7 +912,7 @@ stamps/run-netx-unit-tests.stamp: stamps
> >> done ; \
> >> cd $(NETX_UNIT_TEST_DIR) ; \
> >> class_names=`cat $(UNIT_CLASS_NAMES)` ; \
> >> -
> >> CLASSPATH=$(NETX_DIR)/lib/classes.jar:$(JUNIT_JAR):$(JUNIT_RUNNER_JAR):$(TEST_EXTENSIONS_DIR):.
> >> \
> >> +
> >> CLASSPATH=$(NETX_DIR)/lib/classes.jar:$(DESTDIR)$(datadir)/$(PACKAGE_NAME)/plugin.jar:$(JUNIT_JAR):$(JUNIT_RUNNER_JAR):$(TEST_EXTENSIONS_DIR):.
> >> \
> >> $(BOOT_DIR)/bin/java -Xbootclasspath:$(RUNTIME) CommandLine
> >> $$class_names
> >> if WITH_XSLTPROC
> >> $(XSLTPROC) $(TESTS_SRCDIR)/$(REPORT_STYLES_DIRNAME)/logs.xsl
> >> $(NETX_UNIT_TEST_DIR)/ServerAccess-logs.xml >
> >> $(TESTS_DIR)/logs_unit.html
> >> diff --git a/netx-dist-tests-whitelist b/netx-dist-tests-whitelist
> >> --- a/netx-dist-tests-whitelist
> >> +++ b/netx-dist-tests-whitelist
> >> @@ -1,1 +1,1 @@
> >> -.*
> >> +Applet.*
> >> diff --git a/plugin/icedteanp/IcedTeaNPPlugin.cc
> >> b/plugin/icedteanp/IcedTeaNPPlugin.cc
> >> --- a/plugin/icedteanp/IcedTeaNPPlugin.cc
> >> +++ b/plugin/icedteanp/IcedTeaNPPlugin.cc
> >> @@ -229,8 +229,7 @@ static gboolean plugin_out_pipe_callback
> >> GIOCondition condition,
> >> gpointer plugin_data);
> >> static NPError plugin_start_appletviewer (ITNPPluginData* data);
> >> -static gchar* plugin_create_applet_tag (int16_t argc, char* argn[],
> >> - char* argv[]);
> >> +static std::string plugin_parameters_string (int argc, char* argn[],
> >> char* argv[]);
> >> static void plugin_stop_appletviewer ();
> >> // Uninitialize ITNPPluginData structure
> >> static void plugin_data_destroy (NPP instance);
> >> @@ -347,7 +346,6 @@ ITNP_New (NPMIMEType pluginType, NPP ins
> >>
> >> gchar* documentbase = NULL;
> >> gchar* read_message = NULL;
> >> - gchar* applet_tag = NULL;
> >> gchar* cookie_info = NULL;
> >>
> >> NPObject* npPluginObj = NULL;
> >> @@ -395,11 +393,10 @@ ITNP_New (NPMIMEType pluginType, NPP ins
> >> documentbase = plugin_get_documentbase (instance);
> >> if (documentbase && argc != 0)
> >> {
> >> - // Send applet tag message to appletviewer.
> >> - applet_tag = plugin_create_applet_tag (argc, argn, argv);
> >> -
> >> - data->applet_tag = (gchar*)
> >> malloc(strlen(applet_tag)*sizeof(gchar) +
> >> strlen(documentbase)*sizeof(gchar) + 32);
> >> - g_sprintf(data->applet_tag, "tag %s %s", documentbase,
> >> applet_tag);
> >> + // Send parameters to appletviewer.
> >> + std::string params_string = plugin_parameters_string(argc,
> >> argn, argv);
> >> +
> >> + data->parameters_string = g_strdup_printf("tag %s %s",
> >> documentbase, params_string.c_str());
> >>
> >> data->is_applet_instance = true;
> >> }
> >> @@ -424,33 +421,7 @@ ITNP_New (NPMIMEType pluginType, NPP ins
> >>
> >> instance->pdata = data;
> >>
> >> - goto cleanup_done;
> >> -
> >> - cleanup_appletviewer_mutex:
> >> - g_mutex_free (data->appletviewer_mutex);
> >> - data->appletviewer_mutex = NULL;
> >> -
> >> - // cleanup_instance_string:
> >> - g_free (data->instance_id);
> >> - data->instance_id = NULL;
> >> -
> >> - // cleanup applet tag:
> >> - g_free (data->applet_tag);
> >> - data->applet_tag = NULL;
> >> -
> >> - // cleanup_data:
> >> - // Eliminate back-pointer to plugin instance.
> >> - data->owner = NULL;
> >> - (*browser_functions.memfree) (data);
> >> - data = NULL;
> >> -
> >> - // Initialization failed so return a NULL pointer for the browser
> >> - // data.
> >> - instance->pdata = NULL;
> >> -
> >> cleanup_done:
> >> - g_free (applet_tag);
> >> - applet_tag = NULL;
> >> g_free (read_message);
> >> read_message = NULL;
> >> g_free (documentbase);
> >> @@ -834,7 +805,7 @@ ITNP_SetWindow (NPP instance, NPWindow*
> >> // Now we have everything. Send this data to the Java side
> >> plugin_send_initialization_message(
> >> data->instance_id, (gulong) data->window_handle,
> >> - data->window_width, data->window_height,
> >> data->applet_tag);
> >> + data->window_width, data->window_height,
> >> data->parameters_string);
> >>
> >> g_mutex_unlock (data->appletviewer_mutex);
> >>
> >> @@ -1696,157 +1667,66 @@ plugin_start_appletviewer (ITNPPluginDat
> >>
> >> /*
> >> * Replaces certain characters (\r, \n, etc) with HTML escape
> >> equivalents.
> >> - *
> >> - * Return string is allocated on the heap. Caller assumes
> >> responsibility
> >> - * for freeing the memory via free()
> >> */
> >> -static char*
> >> -encode_string(char* to_encode)
> >> -{
> >> -
> >> - // Do nothing for an empty string
> >> - if (to_encode == '\0')
> >> - return to_encode;
> >> -
> >> - // worst case scenario -> all characters are newlines or
> >> - // returns, each of which translates to 5 substitutions
> >> - char* encoded = (char*) calloc(((strlen(to_encode)*5)+1),
> >> sizeof(char));
> >> -
> >> - strcpy(encoded, "");
> >> -
> >> - for (int i=0; i < strlen(to_encode); i++)
> >> +static std::string
> >> +encode_string(const char* to_encode) {
> >> + std::string encoded;
> >> +
> >> + if (to_encode == NULL)
> >> + {
> >> + return encoded;
> >> + }
> >> +
> >> + size_t length = strlen(to_encode);
> >> + for (int i=0; i < length; i++)
> >> {
> >> if (to_encode[i] == '\r')
> >> - encoded = strcat(encoded, " ");
> >> + encoded += " ";
> >> else if (to_encode[i] == '\n')
> >> - encoded = strcat(encoded, " ");
> >> + encoded += " ";
> >> else if (to_encode[i] == '>')
> >> - encoded = strcat(encoded, ">");
> >> + encoded += ">";
> >> else if (to_encode[i] == '<')
> >> - encoded = strcat(encoded, "<");
> >> + encoded += "<";
> >> else if (to_encode[i] == '&')
> >> - encoded = strcat(encoded, "&");
> >> + encoded += "&";
> >> else if (to_encode[i] == '"')
> >> - encoded = strcat(encoded, """);
> >> + encoded += """;
> >> else
> >> - {
> >> - char* orig_char = (char*) calloc(2, sizeof(char));
> >> - orig_char[0] = to_encode[i];
> >> - orig_char[1] = '\0';
> >> -
> >> - strcat(encoded, orig_char);
> >> -
> >> - free(orig_char);
> >> - orig_char = NULL;
> >> - }
> >> + encoded += to_encode[i];
> >> }
> >>
> >> return encoded;
> >
> > This is probably strangest stuff in patch. NPAPI is returning already
> > decoded strings. Why to encode them again?!?!?!
> >
> > Unless I see something very wrong, get rid of this unnecessary middle
> > step!
> >
> >> }
> >>
> >> -// Build up the applet tag string that we'll send to the applet
> >> -// viewer.
> >> -static gchar*
> >> -plugin_create_applet_tag (int16_t argc, char* argn[], char* argv[])
> >> +// Build a string containing an encoded list of parameters to send to
> >> +// the applet viewer
> >> +static std::string
> >> +plugin_parameters_string (int argc, char* argn[], char* argv[])
> >> {
> >> - PLUGIN_DEBUG ("plugin_create_applet_tag\n");
> >> -
> >> - gchar* applet_tag = g_strdup ("<EMBED ");
> >> - gchar* parameters = g_strdup ("");
> >> -
> >> - for (int16_t i = 0; i < argc; i++)
> >> + PLUGIN_DEBUG ("plugin_parameters_string\n");
> >> +
> >> + std::string parameters;
> >> +
> >> + for (int i = 0; i < argc; i++)
> >> + {
> >> + if (argv[i] != NULL)
> >> {
> >> - gchar* argn_escaped = encode_string(argn[i]);
> >> - gchar* argv_escaped = encode_string(argv[i]);
> >> -
> >> - if (!g_ascii_strcasecmp (argn_escaped, "code"))
> >> - {
> >> - gchar* code = g_strdup_printf ("CODE=\"%s\" ", argv_escaped);
> >> - applet_tag = g_strconcat (applet_tag, code, NULL);
> >> - g_free (code);
> >> - code = NULL;
> >> + std::string name_escaped = encode_string(argn[i]);
> >> + std::string value_escaped = encode_string(argv[i]);
> >> +
> >> + //Encode parameters and send as '"name1" "value1" "name2"
> >> "value2"' etc
> >> + parameters += "\"";
> >> + parameters += name_escaped;
> >> + parameters += "\" ";
> >> + parameters += value_escaped;
> >> + parameters += "\" ";
> >> }
> >> - else if (!g_ascii_strcasecmp (argn_escaped, "java_code"))
> >> - {
> >> - gchar* java_code = g_strdup_printf ("JAVA_CODE=\"%s\" ",
> >> argv_escaped);
> >> - applet_tag = g_strconcat (applet_tag, java_code, NULL);
> >> - g_free (java_code);
> >> - java_code = NULL;
> >> - }
> >> - else if (!g_ascii_strcasecmp (argn_escaped, "codebase"))
> >> - {
> >> - gchar* codebase = g_strdup_printf ("CODEBASE=\"%s\" ",
> >> argv_escaped);
> >> - applet_tag = g_strconcat (applet_tag, codebase, NULL);
> >> - g_free (codebase);
> >> - codebase = NULL;
> >> - }
> >> - else if (!g_ascii_strcasecmp (argn_escaped, "java_codebase"))
> >> - {
> >> - gchar* java_codebase = g_strdup_printf
> >> ("JAVA_CODEBASE=\"%s\" ", argv_escaped);
> >> - applet_tag = g_strconcat (applet_tag, java_codebase, NULL);
> >> - g_free (java_codebase);
> >> - java_codebase = NULL;
> >> - }
> >> - else if (!g_ascii_strcasecmp (argn_escaped, "classid"))
> >> - {
> >> - gchar* classid = g_strdup_printf ("CLASSID=\"%s\" ",
> >> argv_escaped);
> >> - applet_tag = g_strconcat (applet_tag, classid, NULL);
> >> - g_free (classid);
> >> - classid = NULL;
> >> - }
> >> - else if (!g_ascii_strcasecmp (argn_escaped, "archive"))
> >> - {
> >> - gchar* archive = g_strdup_printf ("ARCHIVE=\"%s\" ",
> >> argv_escaped);
> >> - applet_tag = g_strconcat (applet_tag, archive, NULL);
> >> - g_free (archive);
> >> - archive = NULL;
> >> - }
> >> - else if (!g_ascii_strcasecmp (argn_escaped, "java_archive"))
> >> - {
> >> - gchar* java_archive = g_strdup_printf
> >> ("JAVA_ARCHIVE=\"%s\" ", argv_escaped);
> >> - applet_tag = g_strconcat (applet_tag, java_archive, NULL);
> >> - g_free (java_archive);
> >> - java_archive = NULL;
> >> - }
> >> - else if (!g_ascii_strcasecmp (argn_escaped, "width"))
> >> - {
> >> - gchar* width = g_strdup_printf ("width=\"%s\" ",
> >> argv_escaped);
> >> - applet_tag = g_strconcat (applet_tag, width, NULL);
> >> - g_free (width);
> >> - width = NULL;
> >> - }
> >> - else if (!g_ascii_strcasecmp (argn_escaped, "height"))
> >> - {
> >> - gchar* height = g_strdup_printf ("height=\"%s\" ",
> >> argv_escaped);
> >> - applet_tag = g_strconcat (applet_tag, height, NULL);
> >> - g_free (height);
> >> - height = NULL;
> >> - }
> >> - else
> >> - {
> >> -
> >> - if (argv_escaped != '\0')
> >> - {
> >> - parameters = g_strconcat (parameters, "<PARAM
> >> NAME=\"", argn_escaped,
> >> - "\" VALUE=\"", argv_escaped,
> >> "\">", NULL);
> >> - }
> >> - }
> >> -
> >> - free(argn_escaped);
> >> - free(argv_escaped);
> >> -
> >> - argn_escaped = NULL;
> >> - argv_escaped = NULL;
> >> - }
> >> -
> >> - applet_tag = g_strconcat (applet_tag, ">", parameters, "</EMBED>",
> >> NULL);
> >> -
> >> - g_free (parameters);
> >> - parameters = NULL;
> >> -
> >> - PLUGIN_DEBUG ("plugin_create_applet_tag return\n");
> >> -
> >> - return applet_tag;
> >> + }
> >> +
> >> + PLUGIN_DEBUG ("plugin_parameters_string return\n");
> >> +
> >> + return parameters;
> >> }
> >>
> >> // plugin_send_message_to_appletviewer must be called while holding
> >> @@ -2057,8 +1937,8 @@ plugin_data_destroy (NPP instance)
> >> tofree->instance_id = NULL;
> >>
> >> // cleanup applet tag
> >> - g_free (tofree->applet_tag);
> >> - tofree->applet_tag = NULL;
> >> + g_free (tofree->parameters_string);
> >> + tofree->parameters_string = NULL;
> >>
> >> g_free(tofree->source);
> >> tofree->source = NULL;
> >> @@ -2537,7 +2417,7 @@ get_scriptable_object(NPP instance)
> >> // a 0 handle
> >> if (!data->window_handle)
> >> {
> >> - plugin_send_initialization_message(data->instance_id, 0, 0, 0,
> >> data->applet_tag);
> >> + plugin_send_initialization_message(data->instance_id, 0, 0, 0,
> >> data->parameters_string);
> >> }
> >>
> >> java_result = java_request.getAppletObjectInstance(id_str);
> >> diff --git a/plugin/icedteanp/IcedTeaNPPlugin.h
> >> b/plugin/icedteanp/IcedTeaNPPlugin.h
> >> --- a/plugin/icedteanp/IcedTeaNPPlugin.h
> >> +++ b/plugin/icedteanp/IcedTeaNPPlugin.h
> >> @@ -66,8 +66,8 @@ struct ITNPPluginData
> >> {
> >> // A unique identifier for this plugin window.
> >> gchar* instance_id;
> >> - // The applet tag sent to Java side
> >> - gchar* applet_tag;
> >> + // The parameter list string sent to Java side
> >> + gchar* parameters_string;
> >> // Mutex to protect appletviewer_alive.
> >> GMutex* appletviewer_mutex;
> >> // Back-pointer to the plugin instance to which this data belongs.
> >> diff --git
> >> a/plugin/icedteanp/java/sun/applet/PluginAppletAttributes.java
> >> b/plugin/icedteanp/java/sun/applet/PluginAppletAttributes.java
> >> new file mode 100644
> >> --- /dev/null
> >> +++ b/plugin/icedteanp/java/sun/applet/PluginAppletAttributes.java
> >> @@ -0,0 +1,169 @@
> >> +/* PluginAppletAttributes -- Provides parsing for applet attributes
> >> + Copyright (C) 2012 Red Hat
> >> +
> >> +This file is part of IcedTea.
> >> +
> >> +IcedTea is free software; you can redistribute it and/or modify
> >> +it under the terms of the GNU General Public License as published by
> >> +the Free Software Foundation; either version 2, or (at your option)
> >> +any later version.
> >> +
> >> +IcedTea is distributed in the hope that it will be useful, but
> >> +WITHOUT ANY WARRANTY; without even the implied warranty of
> >> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> >> +General Public License for more details.
> >> +
> >> +You should have received a copy of the GNU General Public License
> >> +along with IcedTea; see the file COPYING. If not, write to the
> >> +Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
> >> Boston, MA
> >> +02110-1301 USA.
> >> +
> >> +Linking this library statically or dynamically with other modules is
> >> +making a combined work based on this library. Thus, the terms and
> >> +conditions of the GNU General Public License cover the whole
> >> +combination.
> >> +
> >> +As a special exception, the copyright holders of this library give you
> >> +permission to link this library with independent modules to produce an
> >> +executable, regardless of the license terms of these independent
> >> +modules, and to copy and distribute the resulting executable under
> >> +terms of your choice, provided that you also meet, for each linked
> >> +independent module, the terms and conditions of the license of that
> >> +module. An independent module is a module which is not derived from
> >> +or based on this library. If you modify this library, you may extend
> >> +this exception to your version of the library, but you are not
> >> +obligated to do so. If you do not wish to do so, delete this
> >> +exception statement from your version. */
> >> +
> >> +package sun.applet;
> >> +
> >> +import java.io.PrintStream;
> >> +import java.util.ArrayList;
> >> +import java.util.Enumeration;
> >> +import java.util.Hashtable;
> >> +import java.util.List;
> >> +
> >> +/**
> >> + * Provides parsing for applet attributes passed from the C++ side
> >> as quoted,
> >> + * encoded strings. This class is not meant to be initialized
> >
> > No author in ITW sources please:)
> >
> > Please, mke this class as classical object. You can still keep some
> > static (or singleton) utility methods here, but prefer more object way:
> >
> > Hashtable<String, String> atts =
> > PluginAppletAttributes.getPArser().parse(width, height, tag);
> > or
> > Hashtable<String, String> atts = new
> > PluginAppletAttributes().parse(width, height, tag);
> >
> > Although I would like to recomand to wrap the Hashtable<String,
> > String> to Object. Eg ParsedAppletTag. so the above methods wirr
> > return this one.
> > One will thenbe able to access parsedAppletTag.getKey() and will be
> > also able to recieve something else then Strings
> >
> > And again. Get rid of redundant decoding!
> >
> >
> >> + * @author Adam Domurad
> >> + */
> >> +class PluginAppletAttributes {
> >> + /**
> >> + * Decodes the string (converts html escapes into proper
> >> characters)
> >> + *
> >> + * @param toDecode The string to decode
> >> + * @return The decoded string
> >> + */
> >> + static String decodeString(String toDecode) {
> >> +
> >> + toDecode = toDecode.replace(">", ">");
> >> + toDecode = toDecode.replace("<", "<");
> >> + toDecode = toDecode.replace(" ", "\n");
> >> + toDecode = toDecode.replace(" ", "\r");
> >> + toDecode = toDecode.replace(""", "\"");
> >> + toDecode = toDecode.replace("&", "&");
> >> +
> >> + return toDecode;
> >> + }
> >> +
> >> + static final boolean isInt(String s) {
> >> + try {
> >> + Integer.parseInt(s);
> >> + return true;
> >> + } catch (NumberFormatException e) {
> >> + return false;
> >> + }
> >> + }
> >
> > :DDD no! :D Although this give sense, I would not recommand usage of
> > exception for directing the flow of code :)
> > My suggestion will be matches \d+ ;)
> > I have also some susspiccion that there is ssDigit method somewhere.
> >
> >> +
> >> + static List<String> extractQuotedStrings(String s) {
> >> + List<String> strs = new ArrayList<String>();
> >> + int idx = 0;
> >> +
> >> + while (idx < s.length()){
> >> + // Move to character after starting quote
> >> + idx = s.indexOf('"', idx) + 1;
> >> + if (idx <= 0) break;
> >> +
> >> + // Find end quote
> >> + int endIdx = s.indexOf('"', idx);
> >> + strs.add(s.substring(idx, endIdx));
> >> + idx = endIdx + 1;
> >> + }
> >> +
> >> + return strs;
> >> + }
> >> +
> >> + static Hashtable<String, String> parseQuotedKeyValuePairs(String
> >> keyvalString) {
> >> + List<String> strs = extractQuotedStrings(keyvalString);
> >> + Hashtable<String, String> attributes = new
> >> Hashtable<String,String>();
> >> +
> >> + for (int i = 0; i < strs.size(); i += 2) {
> >> + String key = decodeString(strs.get(i)).toLowerCase();
> >> + String value = decodeString(strs.get(i + 1));
> >> + attributes.put(key, value);
> >> + }
> >> +
> >> + return attributes;
> >> + }
> >> +
> >> + /**
> >> + * Generates a hashtable of applet attributes, given a string
> >> containing
> >> + * parameters in quotes.
> >> + *
> >> + * @param width default applet width
> >> + * @param height default applet height
> >> + * @param parameterString the parameters
> >> + * @return the attributes in a hash table
> >> + */
> >> + static public Hashtable<String, String> parse(String width,
> >> + String height, String parameterString) {
> >> + Hashtable<String, String> atts =
> >> parseQuotedKeyValuePairs(parameterString);
> >> +
> >> + // If there is a classid and no code tag present, transform
> >> it to code tag
> >> + if (atts.get("code") == null && atts.get("classid") != null
> >> + && !(atts.get("classid")).startsWith("clsid:")) {
> >> + atts.put("code", atts.get("classid"));
> >> + }
> >> +
> >> + // remove java: from code tag
> >> + if (atts.get("code") != null &&
> >> (atts.get("code")).startsWith("java:")) {
> >> + atts.put("code", (atts.get("code")).substring(5));
> >> + }
> >> +
> >> + // java_* aliases override older names:
> >> +
> >> //http://java.sun.com/j2se/1.4.2/docs/guide/plugin/developer_guide/using_tags.html#in-nav
> >> + if (atts.get("java_code") != null) {
> >> + atts.put("code", (atts.get("java_code")));
> >> + }
> >> +
> >> + if (atts.get("java_codebase") != null) {
> >> + atts.put("codebase", (atts.get("java_codebase")));
> >> + }
> >> +
> >> + if (atts.get("java_archive") != null) {
> >> + atts.put("archive", (atts.get("java_archive")));
> >> + }
> >> +
> >> + if (atts.get("java_object") != null) {
> >> + atts.put("object", (atts.get("java_object")));
> >> + }
> >> +
> >> + if (atts.get("java_type") != null) {
> >> + atts.put("type", (atts.get("java_type")));
> >> + }
> >> +
> >> + if (atts.get("width") == null || !isInt(atts.get("width"))) {
> >> + atts.put("width", width);
> >> + }
> >> +
> >> + if (atts.get("height") == null || !isInt(atts.get("height"))) {
> >> + atts.put("height", height);
> >> + }
> >> +
> >> + if (atts.get("code") == null && atts.get("object") == null) {
> >> + return null;
> >> + }
> >> + return atts;
> >> + }
> >> +}
> >> diff --git a/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
> >> b/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
> >> --- a/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
> >> +++ b/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
> >> @@ -86,6 +86,7 @@ import java.lang.reflect.InvocationTarge
> >> import java.net.SocketPermission;
> >> import java.net.URI;
> >> import java.net.URL;
> >> +import java.net.URLConnection;
> >> import java.security.AccessController;
> >> import java.security.AllPermission;
> >> import java.security.PrivilegedAction;
> >> @@ -286,6 +287,14 @@ public class PluginAppletViewer extends
> >> PRE_INIT, INIT_COMPLETE, REFRAME_COMPLETE, INACTIVE, DESTROYED
> >> };
> >>
> >> + /* values used for placement of AppletViewer's frames */
> >> + private static final int XDELTA = 30;
> >> + private static final int YDELTA = XDELTA;
> >> +
> >> + private static int x = 0;
> >> + private static int y = 0;
> >> + private static int ydisp = 1;
> >> +
> >> /**
> >> * The panel in which the applet is being displayed.
> >> */
> >> @@ -488,10 +497,37 @@ public class PluginAppletViewer extends
> >> "DocumentBase = ",
> >> documentBase, "\n",
> >> "Tag = ", tag);
> >>
> >> - PluginAppletViewer.parse
> >> - (identifier, handle, width,
> >> height,
> >> - new StringReader(tag),
> >> - new URL(documentBase));
> >> + PluginAppletPanelFactory factory = new
> >> PluginAppletPanelFactory();
> >> + AppletMessageHandler amh = new
> >> AppletMessageHandler("appletviewer");
> >> + URL url = new URL(documentBase);
> >> + URLConnection conn = url.openConnection();
> >> + /* The original URL may have been redirected - this
> >> + * sets it to whatever URL/codebase we ended up getting
> >> + */
> >> + url = conn.getURL();
> >> +
> >
> > I would like to see thios in different method. Also the ydisp remains
> > questionable. Imho with yournew approach it will lead to complete
> > removal of it (i'm really afraid of case like three applets per page,
> > and several such a pages)
> >
> >> + Hashtable<String, String> atts =
> >> PluginAppletAttributes.parse(width, height, tag);
> >> +
> >> + if (atts == null) {
> >> + String embedRequiresCode =
> >> amh.getMessage("parse.warning.embed.requirescode");
> >> + System.out.println(embedRequiresCode);
> >> +
> >> + throw new RuntimeException(embedRequiresCode);
> >> + }
> >> +
> >> + // Let user know we are starting up
> >> + streamhandler.write("instance " + identifier + "
> >> status " + amh.getMessage("status.start"));
> >> + factory.createPanel(streamhandler, identifier,
> >> handle, x, y, url, atts);
> >> +
> >> + x += XDELTA;
> >> + y += YDELTA;
> >> + // make sure we don't go too far!
> >> + Dimension d =
> >> Toolkit.getDefaultToolkit().getScreenSize();
> >> + if ((x > d.width - 300) || (y > d.height - 300)) {
> >> + x = 0;
> >> + y = 2 * ydisp * YDELTA;
> >> + ydisp++;
> >> + }
> >>
> >> long maxTimeToSleep = APPLET_TIMEOUT;
> >> appletsLock.lock();
> >> @@ -1423,24 +1459,6 @@ public class PluginAppletViewer extends
> >> }
> >>
> >> /**
> >> - * Decodes the string (converts html escapes into proper
> >> characters)
> >> - *
> >> - * @param toDecode The string to decode
> >> - * @return The decoded string
> >> - */
> >> - public static String decodeString(String toDecode) {
> >> -
> >> - toDecode = toDecode.replace(">", ">");
> >> - toDecode = toDecode.replace("<", "<");
> >> - toDecode = toDecode.replace("&", "&");
> >> - toDecode = toDecode.replace(" ", "\n");
> >> - toDecode = toDecode.replace(" ", "\r");
> >> - toDecode = toDecode.replace(""", "\"");
> >> -
> >> - return toDecode;
> >> - }
> >> -
> >> - /**
> >> * System parameters.
> >> */
> >> static Hashtable<String, String> systemParam = new
> >> Hashtable<String, String>();
> >> @@ -1457,67 +1475,6 @@ public class PluginAppletViewer extends
> >> }
> >>
> >> /**
> >> - * Print the HTML tag.
> >> - */
> >> - public static void printTag(PrintStream out, Hashtable<String,
> >> String> atts) {
> >> - out.print("<applet");
> >> -
> >> - String v = atts.get("codebase");
> >> - if (v != null) {
> >> - out.print(" codebase=\"" + v + "\"");
> >> - }
> >> -
> >> - v = atts.get("code");
> >> - if (v == null) {
> >> - v = "applet.class";
> >> - }
> >> - out.print(" code=\"" + v + "\"");
> >> - v = atts.get("width");
> >> - if (v == null) {
> >> - v = "150";
> >> - }
> >> - out.print(" width=" + v);
> >> -
> >> - v = atts.get("height");
> >> - if (v == null) {
> >> - v = "100";
> >> - }
> >> - out.print(" height=" + v);
> >> -
> >> - v = atts.get("name");
> >> - if (v != null) {
> >> - out.print(" name=\"" + v + "\"");
> >> - }
> >> - out.println(">");
> >> -
> >> - // A very slow sorting algorithm
> >> - int len = atts.size();
> >> - String params[] = new String[len];
> >> - len = 0;
> >> - for (Enumeration<String> e = atts.keys();
> >> e.hasMoreElements();) {
> >> - String param = e.nextElement();
> >> - int i = 0;
> >> - for (; i < len; i++) {
> >> - if (params[i].compareTo(param) >= 0) {
> >> - break;
> >> - }
> >> - }
> >> - System.arraycopy(params, i, params, i + 1, len - i);
> >> - params[i] = param;
> >> - len++;
> >> - }
> >> -
> >> - for (int i = 0; i < len; i++) {
> >> - String param = params[i];
> >> - if (systemParam.get(param) == null) {
> >> - out.println("<param name=" + param +
> >> - " value=\"" + atts.get(param) + "\">");
> >> - }
> >> - }
> >> - out.println("</applet>");
> >> - }
> >> -
> >> - /**
> >> * Make sure the atrributes are uptodate.
> >> */
> >> public void updateAtts() {
> >> @@ -1673,412 +1630,6 @@ public class PluginAppletViewer extends
> >> return appletPanels.size();
> >> }
> >>
> >> - /**
> >> - * Scan spaces.
> >> - */
> >> - public static void skipSpace(int[] c, Reader in) throws
> >> IOException {
> >> - while ((c[0] >= 0) &&
> >> - ((c[0] == ' ') || (c[0] == '\t') || (c[0] == '\n')
> >> || (c[0] == '\r'))) {
> >> - c[0] = in.read();
> >> - }
> >> - }
> >> -
> >> - /**
> >> - * Scan identifier
> >> - */
> >> - public static String scanIdentifier(int[] c, Reader in) throws
> >> IOException {
> >> - StringBuilder buf = new StringBuilder();
> >> -
> >> - if (c[0] == '!') {
> >> - // Technically, we should be scanning for '!--' but we
> >> are reading
> >> - // from a stream, and there is no way to peek ahead.
> >> That said,
> >> - // a ! at this point can only mean comment here afaik,
> >> so we
> >> - // should be okay
> >> - skipComment(c, in);
> >> - return "";
> >> - }
> >> -
> >> - while (true) {
> >> - if (((c[0] >= 'a') && (c[0] <= 'z')) ||
> >> - ((c[0] >= 'A') && (c[0] <= 'Z')) ||
> >> - ((c[0] >= '0') && (c[0] <= '9')) || (c[0] ==
> >> '_')) {
> >> - buf.append((char) c[0]);
> >> - c[0] = in.read();
> >> - } else {
> >> - return buf.toString();
> >> - }
> >> - }
> >> - }
> >> -
> >> - public static void skipComment(int[] c, Reader in) throws
> >> IOException {
> >> - StringBuilder buf = new StringBuilder();
> >> - boolean commentHeaderPassed = false;
> >> - c[0] = in.read();
> >> - buf.append((char) c[0]);
> >> -
> >> - while (true) {
> >> - if (c[0] == '-' && (c[0] = in.read()) == '-') {
> >> - buf.append((char) c[0]);
> >> - if (commentHeaderPassed) {
> >> - // -- encountered ... is > next?
> >> - if ((c[0] = in.read()) == '>') {
> >> - buf.append((char) c[0]);
> >> -
> >> - PluginDebug.debug("Comment skipped: ",
> >> buf.toString());
> >> -
> >> - // comment skipped.
> >> - return;
> >> - }
> >> - } else {
> >> - // first -- is part of <!-- ... , just mark that
> >> we have passed it
> >> - commentHeaderPassed = true;
> >> - }
> >> -
> >> - } 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());
> >> - return;
> >> - }
> >> -
> >> - c[0] = in.read();
> >> - buf.append((char) c[0]);
> >> - }
> >> - }
> >> -
> >> - /**
> >> - * Scan tag
> >> - */
> >> - public static Hashtable<String, String> scanTag(int[] c, Reader
> >> in) throws IOException {
> >> - Hashtable<String, String> atts = new Hashtable<String,
> >> String>();
> >> - skipSpace(c, in);
> >> - while (c[0] >= 0 && c[0] != '>') {
> >> - String att = decodeString(scanIdentifier(c, in));
> >> - String val = "";
> >> - skipSpace(c, in);
> >> - if (c[0] == '=') {
> >> - int quote = -1;
> >> - c[0] = in.read();
> >> - skipSpace(c, in);
> >> - if ((c[0] == '\'') || (c[0] == '\"')) {
> >> - quote = c[0];
> >> - c[0] = in.read();
> >> - }
> >> - StringBuilder buf = new StringBuilder();
> >> - while ((c[0] > 0) &&
> >> - (((quote < 0) && (c[0] != ' ') && (c[0] !=
> >> '\t') &&
> >> - (c[0] != '\n') && (c[0] != '\r') &&
> >> (c[0] != '>'))
> >> - || ((quote >= 0) && (c[0] != quote)))) {
> >> - buf.append((char) c[0]);
> >> - c[0] = in.read();
> >> - }
> >> - if (c[0] == quote) {
> >> - c[0] = in.read();
> >> - }
> >> - skipSpace(c, in);
> >> - val = decodeString(buf.toString());
> >> - }
> >> -
> >> - PluginDebug.debug("PUT ", att, " = '", val, "'");
> >> - atts.put(att.toLowerCase(java.util.Locale.ENGLISH), val);
> >> -
> >> - while (true) {
> >> - if ((c[0] == '>') || (c[0] < 0) ||
> >> - ((c[0] >= 'a') && (c[0] <= 'z')) ||
> >> - ((c[0] >= 'A') && (c[0] <= 'Z')) ||
> >> - ((c[0] >= '0') && (c[0] <= '9')) || (c[0] ==
> >> '_'))
> >> - break;
> >> - c[0] = in.read();
> >> - }
> >> - //skipSpace(in);
> >> - }
> >> - return atts;
> >> - }
> >> -
> >> - // private static final == inline
> >> - private static final boolean isInt(Object o) {
> >> - boolean isInt = false;
> >> - try {
> >> - Integer.parseInt((String) o);
> >> - isInt = true;
> >> - } catch (Exception e) {
> >> - // don't care
> >> - }
> >> -
> >> - return isInt;
> >> - }
> >> -
> >> - /* values used for placement of AppletViewer's frames */
> >> - private static int x = 0;
> >> - private static int y = 0;
> >> - private static final int XDELTA = 30;
> >> - private static final int YDELTA = XDELTA;
> >> -
> >> - static String encoding = null;
> >> -
> >> - /**
> >> - * Scan an html file for <applet> tags
> >> - */
> >> - public static void parse(int identifier, long handle, String
> >> width, String height, Reader in, URL url, String enc)
> >> - throws IOException {
> >> - encoding = enc;
> >> - parse(identifier, handle, width, height, in, url,
> >> System.out, new PluginAppletPanelFactory());
> >> - }
> >> -
> >> - public static void parse(int identifier, long handle, String
> >> width, String height, Reader in, URL url)
> >> - throws PrivilegedActionException {
> >> -
> >> - final int fIdentifier = identifier;
> >> - final long fHandle = handle;
> >> - final String fWidth = width;
> >> - final String fHeight = height;
> >> - final Reader fIn = in;
> >> - final URL fUrl = url;
> >> - AccessController.doPrivileged(new
> >> PrivilegedExceptionAction<Void>() {
> >> - public Void run() throws IOException {
> >> - parse(fIdentifier, fHandle, fWidth, fHeight, fIn, fUrl,
> >> - System.out, new PluginAppletPanelFactory());
> >> - return null;
> >> - }
> >> - });
> >> - }
> >> -
> >> - @SuppressWarnings("unused")
> >> - public static void parse(int identifier, long handle, String width,
> >> - String height, Reader in, URL url,
> >> - PrintStream statusMsgStream,
> >> - PluginAppletPanelFactory factory)
> >> - throws IOException {
> >> - boolean isObjectTag = false;
> >> - boolean objectTagAlreadyParsed = false;
> >> -
> >> - // The current character
> >> - // FIXME: This is an evil hack to force pass-by-reference.. the
> >> - // parsing code needs to be rewritten from scratch to
> >> prevent such
> >> - //a need
> >> - int[] c = new int[1];
> >> -
> >> - // warning messages
> >> - String requiresNameWarning =
> >> amh.getMessage("parse.warning.requiresname");
> >> - String paramOutsideWarning =
> >> amh.getMessage("parse.warning.paramoutside");
> >> - String appletRequiresCodeWarning =
> >> amh.getMessage("parse.warning.applet.requirescode");
> >> - String appletRequiresHeightWarning =
> >> amh.getMessage("parse.warning.applet.requiresheight");
> >> - String appletRequiresWidthWarning =
> >> amh.getMessage("parse.warning.applet.requireswidth");
> >> - String objectRequiresCodeWarning =
> >> amh.getMessage("parse.warning.object.requirescode");
> >> - String objectRequiresHeightWarning =
> >> amh.getMessage("parse.warning.object.requiresheight");
> >> - String objectRequiresWidthWarning =
> >> amh.getMessage("parse.warning.object.requireswidth");
> >> - String embedRequiresCodeWarning =
> >> amh.getMessage("parse.warning.embed.requirescode");
> >> - String embedRequiresHeightWarning =
> >> amh.getMessage("parse.warning.embed.requiresheight");
> >> - String embedRequiresWidthWarning =
> >> amh.getMessage("parse.warning.embed.requireswidth");
> >> - String appNotLongerSupportedWarning =
> >> amh.getMessage("parse.warning.appnotLongersupported");
> >> -
> >> - java.net.URLConnection conn = url.openConnection();
> >> - /* The original URL may have been redirected - this
> >> - * sets it to whatever URL/codebase we ended up getting
> >> - */
> >> - url = conn.getURL();
> >> -
> >> - int ydisp = 1;
> >> - Hashtable<String, String> atts = null;
> >> -
> >> - while (true) {
> >> - c[0] = in.read();
> >> - if (c[0] == -1)
> >> - break;
> >> -
> >> - if (c[0] == '<') {
> >> - c[0] = in.read();
> >> - if (c[0] == '/') {
> >> - c[0] = in.read();
> >> - String nm = scanIdentifier(c, in);
> >> - if (nm.equalsIgnoreCase("applet") ||
> >> - nm.equalsIgnoreCase("object") ||
> >> - nm.equalsIgnoreCase("embed")) {
> >> -
> >> - // We can't test for a code tag until </OBJECT>
> >> - // because it is a parameter, not an attribute.
> >> - if (isObjectTag) {
> >> - if (atts.get("code") == null &&
> >> atts.get("object") == null) {
> >> - statusMsgStream.println(objectRequiresCodeWarning);
> >> - atts = null;
> >> - }
> >> - }
> >> -
> >> - if (atts != null) {
> >> - // XXX 5/18 In general this code just
> >> simply
> >> - // shouldn't be part of parsing. It's
> >> presence
> >> - // causes things to be a little too much
> >> of a
> >> - // hack.
> >> -
> >> - // Let user know we are starting up
> >> - streamhandler.write("instance " +
> >> identifier + " status " + amh.getMessage("status.start"));
> >> - factory.createPanel(streamhandler,
> >> identifier, handle, x, y, url, atts);
> >> -
> >> - x += XDELTA;
> >> - y += YDELTA;
> >> - // make sure we don't go too far!
> >> - Dimension d =
> >> Toolkit.getDefaultToolkit().getScreenSize();
> >> - if ((x > d.width - 300) || (y > d.height
> >> - 300)) {
> >> - x = 0;
> >> - y = 2 * ydisp * YDELTA;
> >> - ydisp++;
> >> - }
> >> - }
> >> - atts = null;
> >> - isObjectTag = false;
> >> - }
> >> - } else {
> >> - String nm = scanIdentifier(c, in);
> >> - if (nm.equalsIgnoreCase("param")) {
> >> - Hashtable<String, String> t = scanTag(c, in);
> >> - String att = t.get("name");
> >> -
> >> - if (att == null) {
> >> - statusMsgStream.println(requiresNameWarning);
> >> - } else {
> >> - String val = t.get("value");
> >> - if (val == null) {
> >> - statusMsgStream.println(requiresNameWarning);
> >> - } else {
> >> - PluginDebug.debug("PUT ", att, " =
> >> ", val);
> >> - atts.put(att.toLowerCase(), val);
> >> - }
> >> - }
> >> - } else if (nm.equalsIgnoreCase("applet")) {
> >> - atts = scanTag(c, in);
> >> -
> >> - // If there is a classid and no code tag
> >> present, transform it to code tag
> >> - if (atts.get("code") == null &&
> >> atts.get("classid") != null
> >> - &&
> >> !(atts.get("classid")).startsWith("clsid:")) {
> >> - atts.put("code", atts.get("classid"));
> >> - }
> >> -
> >> - // remove java: from code tag
> >> - if (atts.get("code") != null &&
> >> (atts.get("code")).startsWith("java:")) {
> >> - atts.put("code",
> >> (atts.get("code")).substring(5));
> >> - }
> >> -
> >> - if (atts.get("code") == null &&
> >> atts.get("object") == null) {
> >> - statusMsgStream.println(appletRequiresCodeWarning);
> >> - atts = null;
> >> - }
> >> -
> >> - if (atts.get("width") == null ||
> >> !isInt(atts.get("width"))) {
> >> - atts.put("width", width);
> >> - }
> >> -
> >> - if (atts.get("height") == null ||
> >> !isInt(atts.get("height"))) {
> >> - atts.put("height", height);
> >> - }
> >> - } else if (nm.equalsIgnoreCase("object")) {
> >> - isObjectTag = true;
> >> -
> >> - // Once code is set, additional nested
> >> objects are ignored
> >> - if (!objectTagAlreadyParsed) {
> >> - objectTagAlreadyParsed = true;
> >> - atts = scanTag(c, in);
> >> - }
> >> -
> >> - // If there is a classid and no code tag
> >> present, transform it to code tag
> >> - if (atts.get("code") == null &&
> >> atts.get("classid") != null
> >> - &&
> >> !(atts.get("classid")).startsWith("clsid:")) {
> >> - atts.put("code", atts.get("classid"));
> >> - }
> >> -
> >> - // remove java: from code tag
> >> - if (atts.get("code") != null &&
> >> (atts.get("code")).startsWith("java:")) {
> >> - atts.put("code",
> >> (atts.get("code")).substring(5));
> >> - }
> >> -
> >> - // java_* aliases override older names:
> >> -
> >> //http://java.sun.com/j2se/1.4.2/docs/guide/plugin/developer_guide/using_tags.html#in-ie
> >> - if (atts.get("java_code") != null) {
> >> - atts.put("code", (atts.get("java_code")));
> >> - }
> >> -
> >> - if (atts.containsKey("code")) {
> >> - objectTagAlreadyParsed = true;
> >> - }
> >> -
> >> - if (atts.get("java_codebase") != null) {
> >> - atts.put("codebase",
> >> (atts.get("java_codebase")));
> >> - }
> >> -
> >> - if (atts.get("java_archive") != null) {
> >> - atts.put("archive",
> >> (atts.get("java_archive")));
> >> - }
> >> -
> >> - if (atts.get("java_object") != null) {
> >> - atts.put("object",
> >> (atts.get("java_object")));
> >> - }
> >> -
> >> - if (atts.get("java_type") != null) {
> >> - atts.put("type", (atts.get("java_type")));
> >> - }
> >> -
> >> - if (atts.get("width") == null ||
> >> !isInt(atts.get("width"))) {
> >> - atts.put("width", width);
> >> - }
> >> -
> >> - if (atts.get("height") == null ||
> >> !isInt(atts.get("height"))) {
> >> - atts.put("height", height);
> >> - }
> >> - } else if (nm.equalsIgnoreCase("embed")) {
> >> - atts = scanTag(c, in);
> >> -
> >> - // If there is a classid and no code tag
> >> present, transform it to code tag
> >> - if (atts.get("code") == null &&
> >> atts.get("classid") != null
> >> - &&
> >> !(atts.get("classid")).startsWith("clsid:")) {
> >> - atts.put("code", atts.get("classid"));
> >> - }
> >> -
> >> - // remove java: from code tag
> >> - if (atts.get("code") != null &&
> >> (atts.get("code")).startsWith("java:")) {
> >> - atts.put("code",
> >> (atts.get("code")).substring(5));
> >> - }
> >> -
> >> - // java_* aliases override older names:
> >> -
> >> //http://java.sun.com/j2se/1.4.2/docs/guide/plugin/developer_guide/using_tags.html#in-nav
> >> - if (atts.get("java_code") != null) {
> >> - atts.put("code", (atts.get("java_code")));
> >> - }
> >> -
> >> - if (atts.get("java_codebase") != null) {
> >> - atts.put("codebase",
> >> (atts.get("java_codebase")));
> >> - }
> >> -
> >> - if (atts.get("java_archive") != null) {
> >> - atts.put("archive",
> >> (atts.get("java_archive")));
> >> - }
> >> -
> >> - if (atts.get("java_object") != null) {
> >> - atts.put("object",
> >> (atts.get("java_object")));
> >> - }
> >> -
> >> - if (atts.get("java_type") != null) {
> >> - atts.put("type", (atts.get("java_type")));
> >> - }
> >> -
> >> - if (atts.get("code") == null &&
> >> atts.get("object") == null) {
> >> - statusMsgStream.println(embedRequiresCodeWarning);
> >> - atts = null;
> >> - }
> >> -
> >> - if (atts.get("width") == null ||
> >> !isInt(atts.get("width"))) {
> >> - atts.put("width", width);
> >> - }
> >> -
> >> - if (atts.get("height") == null ||
> >> !isInt(atts.get("height"))) {
> >> - atts.put("height", height);
> >> - }
> >> -
> >> - }
> >> - }
> >> - }
> >> - }
> >> - in.close();
> >> - }
> >> -
> >> - private static AppletMessageHandler amh = new
> >> AppletMessageHandler("appletviewer");
> >
> >
> > So nice removal :)
> >>
> >> private static void checkConnect(URL url) {
> >> SecurityManager security = System.getSecurityManager();
> >> diff --git
> >> a/tests/netx/unit/sun/applet/PluginAppletAttributesTest.java
> >> b/tests/netx/unit/sun/applet/PluginAppletAttributesTest.java
> >> new file mode 100644
> >> --- /dev/null
> >> +++ b/tests/netx/unit/sun/applet/PluginAppletAttributesTest.java
> >> @@ -0,0 +1,131 @@
> >> +package sun.applet;
> >> +
> >> +import static org.junit.Assert.assertEquals;
> >> +import static org.junit.Assert.assertTrue;
> >> +import static org.junit.Assert.assertFalse;
> >> +
> >> +import java.io.IOException;
> >> +import java.util.Hashtable;
> >> +import java.util.List;
> >> +
> >> +import org.junit.Test;
> >> +
> >> +public class PluginAppletAttributesTest {
> >> +
> >> + @Test
> >> + public void testIsInt() {
> >> + assertFalse(PluginAppletAttributes.isInt("1.0"));
> >> + assertFalse(PluginAppletAttributes.isInt("abc"));
> >> + assertTrue(PluginAppletAttributes.isInt("1"));
> >> + }
> >> +
> >> + @Test
> >> + public void testDecodeString() {
> >> + assertEquals("< <> >",
> >> + PluginAppletAttributes.decodeString("< <>
> >> >"));
> >> + assertEquals("\" \" \" \"",
> >> + PluginAppletAttributes.decodeString("" "
> >> " ""));
> >> + assertEquals("& & &",
> >> + PluginAppletAttributes.decodeString("& &
> >> &"));
> >> + }
> >> +
> >> + @Test
> >> + public void testExtractQuotedStrings() {
> >> + String teststr1 = "\"1\" \"2\" \"3\" \"4\"";
> >> + List<String> extracted1 =
> >> PluginAppletAttributes.extractQuotedStrings(teststr1);
> >> +
> >> + assertEquals(extracted1.size(), 4);
> >> +
> >> + assertEquals("1", extracted1.get(0));
> >> + assertEquals("2", extracted1.get(1));
> >> + assertEquals("3", extracted1.get(2));
> >> + assertEquals("4", extracted1.get(3));
> >> +
> >> + String teststr2 = "\"space\" \"after\" ";
> >> +
> >> + List<String> extracted2 =
> >> PluginAppletAttributes.extractQuotedStrings(teststr2);
> >> + assertEquals(2, extracted2.size());
> >> +
> >> + assertEquals("space", extracted2.get(0));
> >> + assertEquals("after", extracted2.get(1));
> >> +
> >> + }
> >> +
> >> + @Test
> >> + public void testParseQuotedKeyValuePairs() {
> >> + final String quotedPartString = "\"key\" \"val\"
> >> \"KEY&\" \""\"";
> >> + Hashtable<String, String> atts =
> >> PluginAppletAttributes.parseQuotedKeyValuePairs(quotedPartString);
> >> +
> >> + assertEquals(atts.size(), 2);
> >> +
> >> + assertEquals(atts.get("key"), "val");
> >> + //ensure key is lowercased
> >> + assertEquals(atts.get("key&"), "\"");
> >> + }
> >> +
> >> + @Test
> >> + public void testAttributeParseJavaPrefix() {
> >> + // java_* aliases override older names:
> >> +
> >> //http://java.sun.com/j2se/1.4.2/docs/guide/plugin/developer_guide/using_tags.html#in-nav
> >> +
> >> + final String width = "1", height = "1";
> >> + final String codeKeyVal = "\"code\" \"codeValue\" ";
> >> +
> >> + Hashtable<String, String> atts;
> >> +
> >> + atts = PluginAppletAttributes.parse(width, height,
> >> + codeKeyVal + "\"java_code\" \"java_codeValue\" ");
> >> + assertEquals("java_codeValue", atts.get("code"));
> >> +
> >> + atts = PluginAppletAttributes.parse(width, height,
> >> + codeKeyVal + "\"codebase\" \"codebaseValue\"
> >> \"java_codebase\" \"java_codebaseValue\" ");
> >> + assertEquals("java_codebaseValue", atts.get("codebase"));
> >> +
> >> + atts = PluginAppletAttributes.parse(width, height,
> >> + codeKeyVal + "\"archive\" \"archiveValue\"
> >> \"java_archive\" \"java_archiveValue\" ");
> >> + assertEquals("java_archiveValue", atts.get("archive"));
> >> +
> >> + atts = PluginAppletAttributes.parse(width, height,
> >> + codeKeyVal + "\"object\" \"objectValue\"
> >> \"java_object\" \"java_objectValue\" ");
> >> + assertEquals("java_objectValue", atts.get("object"));
> >> +
> >> + atts = PluginAppletAttributes.parse(width, height,
> >> + codeKeyVal + "\"type\" \"typeValue\" \"java_type\"
> >> \"java_typeValue\" ");
> >> + assertEquals("java_typeValue", atts.get("type"));
> >> + }
> >> +
> >> + @Test
> >> + public void testAttributeParseWidthHeightAttributes() {
> >> + final String width = "1", height = "1";
> >> + final String codeKeyVal = "\"code\" \"codeValue\" ";
> >> +
> >> + Hashtable<String, String> atts;
> >> +
> >> + atts = PluginAppletAttributes.parse(width, height, codeKeyVal);
> >> + assertEquals("1", atts.get("width"));
> >> + assertEquals("1", atts.get("height"));
> >> +
> >> + //Test that width height are defaulted to in case of
> >> not-a-number attributes:
> >> + atts = PluginAppletAttributes.parse(width, height,
> >> codeKeyVal + " \"width\" \"NAN\" \"height\" \"NAN\" ");
> >> + assertEquals("1", atts.get("width"));
> >> + assertEquals("1", atts.get("height"));
> >> + }
> >> +
> >> + @Test
> >> + public void testAttributeParseCodeAttribute() {
> >> + final String width = "1", height = "1";
> >> + Hashtable<String, String> atts;
> >> +
> >> + atts = PluginAppletAttributes.parse(width, height,
> >> "\"classid\" \"classidValue\" ");
> >> + assertEquals("classidValue", atts.get("code"));
> >> +
> >> + atts = PluginAppletAttributes.parse(width, height, "\"code\"
> >> \"java:codeValue\" ");
> >> + assertEquals("codeValue", atts.get("code"));
> >> +
> >> + atts = PluginAppletAttributes.parse(width, height,
> >> "\"classid\" \"clsid:classidValue\" ");
> >> + assertEquals(null, atts); //NB: atts == null if theres no
> >> object or code attribute
> >> +
> >> + atts = PluginAppletAttributes.parse(width, height,
> >> "\"object\" \"objectValue\" ");
> >> + assertEquals("objectValue", atts.get("object"));
> >> + }
> >> +}
> >
> >
> > I do not want to be pedantic, but how deep is coverage of those?
> > And probably tehre will come need to test possible new ParsedAppletTAg
> > class.
> >>
> >
> > Thanx for valuable patch!
> >
> > J.
> >
>
> Thanks for the comments, I hope to have addressed them. Attached is
> iteration 2, let me know what you think of the direction it is going.
> This patch introduces net.sourceforge.jnlp.PluginParameters as well as
> sun.applet.PluginParameterParser. I have removed the use of static
> method as suggested.
>
> The decoded was replaced by a simple escaping.
> The wrapped PluginParameter's class was used where I thought was
> logical, please comment especially on the placement of this class where
> Hashtable was previously used.
>
> Thanks for the review & hints,
> - Adam
More information about the distro-pkg-dev
mailing list