You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by jd...@apache.org on 2009/01/03 10:29:02 UTC

svn commit: r730942 - in /wicket/trunk/wicket/src/main/java/org/apache/wicket/markup: ComponentTag.java parser/TagAttributes.java parser/XmlPullParser.java parser/XmlTag.java parser/filter/HeadForceTagIdHandler.java

Author: jdonnerstag
Date: Sat Jan  3 01:29:02 2009
New Revision: 730942

URL: http://svn.apache.org/viewvc?rev=730942&view=rev
Log:
fixed wicket-1380: (Simple)AttributeModifier abuse check

Introduced TagAttribute which extends ValueMap and which does the check. An internal put method is available for e.g. MarkupParser which genuinely needs to add the "id" attribute to the map.

Added:
    wicket/trunk/wicket/src/main/java/org/apache/wicket/markup/parser/TagAttributes.java
Modified:
    wicket/trunk/wicket/src/main/java/org/apache/wicket/markup/ComponentTag.java
    wicket/trunk/wicket/src/main/java/org/apache/wicket/markup/parser/XmlPullParser.java
    wicket/trunk/wicket/src/main/java/org/apache/wicket/markup/parser/XmlTag.java
    wicket/trunk/wicket/src/main/java/org/apache/wicket/markup/parser/filter/HeadForceTagIdHandler.java

Modified: wicket/trunk/wicket/src/main/java/org/apache/wicket/markup/ComponentTag.java
URL: http://svn.apache.org/viewvc/wicket/trunk/wicket/src/main/java/org/apache/wicket/markup/ComponentTag.java?rev=730942&r1=730941&r2=730942&view=diff
==============================================================================
--- wicket/trunk/wicket/src/main/java/org/apache/wicket/markup/ComponentTag.java (original)
+++ wicket/trunk/wicket/src/main/java/org/apache/wicket/markup/ComponentTag.java Sat Jan  3 01:29:02 2009
@@ -213,6 +213,17 @@
 	}
 
 	/**
+	 * A convenient method. The same as getAttributes().getString(name)
+	 * 
+	 * @param name
+	 * @return The attributes value
+	 */
+	public final String getAttribute(String name)
+	{
+		return xmlTag.getAttributes().getString(name);
+	}
+
+	/**
 	 * Get the tag's component id
 	 * 
 	 * @return The component id attribute of this tag
@@ -400,8 +411,8 @@
 	}
 
 	/**
-	 * Copies all internal properties from this tag to <code>dest</code>. This is basically
-	 * cloning without instance creation.
+	 * Copies all internal properties from this tag to <code>dest</code>. This is basically cloning
+	 * without instance creation.
 	 * 
 	 * @param dest
 	 *            tag whose properties will be set
@@ -433,6 +444,7 @@
 	public final void put(final String key, final boolean value)
 	{
 		xmlTag.put(key, value);
+		setModified(true);
 	}
 
 	/**
@@ -445,6 +457,7 @@
 	public final void put(final String key, final int value)
 	{
 		xmlTag.put(key, value);
+		setModified(true);
 	}
 
 	/**
@@ -457,6 +470,7 @@
 	public final void put(String key, CharSequence value)
 	{
 		xmlTag.put(key, value);
+		setModified(true);
 	}
 
 	/**
@@ -469,6 +483,7 @@
 	public final void put(String key, StringValue value)
 	{
 		xmlTag.put(key, value);
+		setModified(true);
 	}
 
 	/**
@@ -479,6 +494,7 @@
 	public final void putAll(final Map<String, Object> map)
 	{
 		xmlTag.putAll(map);
+		setModified(true);
 	}
 
 	/**
@@ -489,6 +505,7 @@
 	public final void remove(String key)
 	{
 		xmlTag.remove(key);
+		setModified(true);
 	}
 
 	/**

Added: wicket/trunk/wicket/src/main/java/org/apache/wicket/markup/parser/TagAttributes.java
URL: http://svn.apache.org/viewvc/wicket/trunk/wicket/src/main/java/org/apache/wicket/markup/parser/TagAttributes.java?rev=730942&view=auto
==============================================================================
--- wicket/trunk/wicket/src/main/java/org/apache/wicket/markup/parser/TagAttributes.java (added)
+++ wicket/trunk/wicket/src/main/java/org/apache/wicket/markup/parser/TagAttributes.java Sat Jan  3 01:29:02 2009
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.markup.parser;
+
+import java.util.Iterator;
+import java.util.Map;
+
+import org.apache.wicket.util.value.ValueMap;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * 
+ */
+public class TagAttributes extends ValueMap
+{
+	/** Log. */
+	static final Logger log = LoggerFactory.getLogger(TagAttributes.class);
+
+	private static final long serialVersionUID = 1L;
+
+	/**
+	 * Constructs empty <code>ValueMap</code>.
+	 */
+	public TagAttributes()
+	{
+		super();
+	}
+
+	/**
+	 * Copy constructor.
+	 * 
+	 * @param map
+	 *            the <code>ValueMap</code> to copy
+	 */
+	public TagAttributes(final Map map)
+	{
+		super();
+		putAll(map);
+	}
+
+	/**
+	 * @see org.apache.wicket.util.value.ValueMap#put(java.lang.String, java.lang.Object)
+	 */
+	@Override
+	public final Object put(String key, Object value)
+	{
+		checkIdAttribute(key);
+		return super.put(key, value);
+	}
+
+	/**
+	 * @param key
+	 */
+	private void checkIdAttribute(String key)
+	{
+		if ((key != null) && (key.equalsIgnoreCase("id")))
+		{
+			log.warn("WARNING: Please use component.setMarkupId(String) to change the tag's 'id' attribute.");
+		}
+	}
+
+	/**
+	 * Modifying the 'id' attribute should be made via Component.setMarkupId(). But the markup
+	 * parser must still be able to add the 'id' attribute without warning.
+	 * 
+	 * @param key
+	 * @param value
+	 * @return The old value
+	 */
+	public final Object putInternal(String key, Object value)
+	{
+		return super.put(key, value);
+	}
+
+	/**
+	 * @see org.apache.wicket.util.value.ValueMap#putAll(java.util.Map)
+	 */
+	@Override
+	public final void putAll(Map map)
+	{
+		Iterator<?> iter = map.keySet().iterator();
+		while (iter.hasNext())
+		{
+			String key = (String)iter.next();
+			checkIdAttribute(key);
+		}
+
+		super.putAll(map);
+	}
+}
\ No newline at end of file

Modified: wicket/trunk/wicket/src/main/java/org/apache/wicket/markup/parser/XmlPullParser.java
URL: http://svn.apache.org/viewvc/wicket/trunk/wicket/src/main/java/org/apache/wicket/markup/parser/XmlPullParser.java?rev=730942&r1=730941&r2=730942&view=diff
==============================================================================
--- wicket/trunk/wicket/src/main/java/org/apache/wicket/markup/parser/XmlPullParser.java (original)
+++ wicket/trunk/wicket/src/main/java/org/apache/wicket/markup/parser/XmlPullParser.java Sat Jan  3 01:29:02 2009
@@ -61,8 +61,8 @@
 	public static final int SPECIAL_TAG = 6;
 
 	/**
-	 * Reads the xml data from an input stream and converts the chars according to its encoding (<?xml
-	 * ... encoding="..." ?>)
+	 * Reads the xml data from an input stream and converts the chars according to its encoding
+	 * (<?xml ... encoding="..." ?>)
 	 */
 	private XmlReader xmlReader;
 
@@ -149,8 +149,7 @@
 			if ((pos == -1) || ((pos + (tagNameLen + 2)) >= input.size()))
 			{
 				throw new ParseException(skipUntilText + " tag not closed (line " +
-						input.getLineNumber() + ", column " + input.getColumnNumber() + ")",
-						startIndex);
+					input.getLineNumber() + ", column " + input.getColumnNumber() + ")", startIndex);
 			}
 
 			lastPos = pos + 2;
@@ -166,7 +165,7 @@
 		if (lastPos == -1)
 		{
 			throw new ParseException("Script tag not closed (line " + input.getLineNumber() +
-					", column " + input.getColumnNumber() + ")", startIndex);
+				", column " + input.getColumnNumber() + ")", startIndex);
 		}
 
 		// Reset the state variable
@@ -222,7 +221,7 @@
 		if (closeBracketIndex == -1)
 		{
 			throw new ParseException("No matching close bracket at position " + openBracketIndex,
-					input.getPosition());
+				input.getPosition());
 		}
 
 		// Get the complete tag text
@@ -232,8 +231,8 @@
 		String tagText = lastText.subSequence(1, lastText.length() - 1).toString();
 		if (tagText.length() == 0)
 		{
-			throw new ParseException("Found empty tag: '<>' at position " + openBracketIndex, input
-					.getPosition());
+			throw new ParseException("Found empty tag: '<>' at position " + openBracketIndex,
+				input.getPosition());
 		}
 
 		// Handle special tags like <!-- and <![CDATA ...
@@ -267,7 +266,7 @@
 			// If open tag and starts with "s" like "script" or "style", than
 			// ...
 			if ((tagText.length() > 5) &&
-					((tagText.charAt(0) == 's') || (tagText.charAt(0) == 'S')))
+				((tagText.charAt(0) == 's') || (tagText.charAt(0) == 'S')))
 			{
 				final String lowerCase = tagText.substring(0, 6).toLowerCase();
 				if (lowerCase.startsWith("script"))
@@ -304,7 +303,7 @@
 		else
 		{
 			throw new ParseException("Malformed tag (line " + input.getLineNumber() + ", column " +
-					input.getColumnNumber() + ")", openBracketIndex);
+				input.getColumnNumber() + ")", openBracketIndex);
 		}
 	}
 
@@ -317,7 +316,7 @@
 	 * @throws ParseException
 	 */
 	private void specialTagHandling(String tagText, final int openBracketIndex,
-			int closeBracketIndex) throws ParseException
+		int closeBracketIndex) throws ParseException
 	{
 		// Handle comments
 		if (tagText.startsWith("!--"))
@@ -330,8 +329,7 @@
 			if (pos == -1)
 			{
 				throw new ParseException("Unclosed comment beginning at line:" +
-						input.getLineNumber() + " column:" + input.getColumnNumber(),
-						openBracketIndex);
+					input.getLineNumber() + " column:" + input.getColumnNumber(), openBracketIndex);
 			}
 
 			pos += 3;
@@ -340,7 +338,7 @@
 
 			// Conditional comment? <!--[if ...]>..<![endif]-->
 			if (tagText.startsWith("!--[if ") && tagText.endsWith("]") &&
-					lastText.toString().endsWith("<![endif]-->"))
+				lastText.toString().endsWith("<![endif]-->"))
 			{
 				// Actually it is no longer a comment. It is now
 				// up to the browser to select the section appropriate.
@@ -377,13 +375,13 @@
 					if (closeBracketIndex == -1)
 					{
 						throw new ParseException("No matching close bracket at line:" +
-								input.getLineNumber() + " column:" + input.getColumnNumber(), input
-								.getPosition());
+							input.getLineNumber() + " column:" + input.getColumnNumber(),
+							input.getPosition());
 					}
 
 					// Get the tagtext between open and close brackets
 					tagText = input.getSubstring(openBracketIndex + 1, closeBracketIndex)
-							.toString();
+						.toString();
 
 					pos1 = closeBracketIndex + 1;
 				}
@@ -497,7 +495,7 @@
 	 *             Resource not found
 	 */
 	public void parse(final CharSequence string) throws IOException,
-			ResourceStreamNotFoundException
+		ResourceStreamNotFoundException
 	{
 		parse(new ByteArrayInputStream(string.toString().getBytes()), null);
 	}
@@ -528,7 +526,7 @@
 	 * @throws ResourceStreamNotFoundException
 	 */
 	public void parse(final InputStream inputStream, final String encoding) throws IOException,
-			ResourceStreamNotFoundException
+		ResourceStreamNotFoundException
 	{
 		try
 		{
@@ -567,6 +565,7 @@
 	 * 
 	 * @see java.lang.Object#toString()
 	 */
+	@Override
 	public String toString()
 	{
 		return input.toString();
@@ -632,10 +631,10 @@
 				final String key = attributeParser.getKey();
 
 				// Put the attribute in the attributes hash
-				if (null != tag.put(key, value))
+				if (null != ((TagAttributes)tag.getAttributes()).putInternal(key, value))
 				{
-					throw new ParseException("Same attribute found twice: " + key, input
-							.getPosition());
+					throw new ParseException("Same attribute found twice: " + key,
+						input.getPosition());
 				}
 
 				// The input has to match exactly (no left over junk after

Modified: wicket/trunk/wicket/src/main/java/org/apache/wicket/markup/parser/XmlTag.java
URL: http://svn.apache.org/viewvc/wicket/trunk/wicket/src/main/java/org/apache/wicket/markup/parser/XmlTag.java?rev=730942&r1=730941&r2=730942&view=diff
==============================================================================
--- wicket/trunk/wicket/src/main/java/org/apache/wicket/markup/parser/XmlTag.java (original)
+++ wicket/trunk/wicket/src/main/java/org/apache/wicket/markup/parser/XmlTag.java Sat Jan  3 01:29:02 2009
@@ -27,6 +27,8 @@
 import org.apache.wicket.util.string.Strings;
 import org.apache.wicket.util.value.IValueMap;
 import org.apache.wicket.util.value.ValueMap;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 
 /**
@@ -37,6 +39,9 @@
  */
 public class XmlTag extends MarkupElement
 {
+	/** Log. */
+	private static final Logger log = LoggerFactory.getLogger(XmlTag.class);
+
 	/** A close tag, like &lt;/TAG&gt;. */
 	public static final Type CLOSE = new Type("CLOSE");
 
@@ -161,11 +166,11 @@
 		{
 			if ((copyOf == this) || (copyOf == null) || (copyOf.attributes == null))
 			{
-				attributes = new ValueMap();
+				attributes = new TagAttributes();
 			}
 			else
 			{
-				attributes = new ValueMap(copyOf.attributes);
+				attributes = new TagAttributes(copyOf.attributes);
 			}
 		}
 		return attributes;
@@ -383,8 +388,8 @@
 	}
 
 	/**
-	 * Copies all internal properties from this tag to <code>dest</code>. This is basically
-	 * cloning without instance creation.
+	 * Copies all internal properties from this tag to <code>dest</code>. This is basically cloning
+	 * without instance creation.
 	 * 
 	 * @param dest
 	 *            tag whose properties will be set

Modified: wicket/trunk/wicket/src/main/java/org/apache/wicket/markup/parser/filter/HeadForceTagIdHandler.java
URL: http://svn.apache.org/viewvc/wicket/trunk/wicket/src/main/java/org/apache/wicket/markup/parser/filter/HeadForceTagIdHandler.java?rev=730942&r1=730941&r2=730942&view=diff
==============================================================================
--- wicket/trunk/wicket/src/main/java/org/apache/wicket/markup/parser/filter/HeadForceTagIdHandler.java (original)
+++ wicket/trunk/wicket/src/main/java/org/apache/wicket/markup/parser/filter/HeadForceTagIdHandler.java Sat Jan  3 01:29:02 2009
@@ -22,14 +22,16 @@
 import org.apache.wicket.markup.MarkupElement;
 import org.apache.wicket.markup.WicketTag;
 import org.apache.wicket.markup.parser.AbstractMarkupFilter;
+import org.apache.wicket.markup.parser.TagAttributes;
 import org.apache.wicket.util.string.AppendingStringBuffer;
 
 
 /**
  * Handler that sets unique tag id for every inline script and style element in &lt;wicket:head&gt;,
- * unless the element already has one. <br/> This is needed to be able to detect multiple ajax
- * header contribution. Tags that are not inline (stript with src attribute set and link with href
- * attribute set) do not require id, because the detection is done by comparing URLs.
+ * unless the element already has one. <br/>
+ * This is needed to be able to detect multiple ajax header contribution. Tags that are not inline
+ * (stript with src attribute set and link with href attribute set) do not require id, because the
+ * detection is done by comparing URLs.
  * <p>
  * Tags with wicket:id are <strong>not processed</strong>. To setOutputWicketId(true) on attached
  * component is developer's responsibility. FIXME: Really? And if so, document properly
@@ -53,7 +55,7 @@
 	 * @param markupFileClass
 	 *            Used to generated the a common prefix for the id
 	 */
-	public HeadForceTagIdHandler(final Class< ? > markupFileClass)
+	public HeadForceTagIdHandler(final Class<?> markupFileClass)
 	{
 		// generate the prefix from class name
 		final AppendingStringBuffer buffer = new AppendingStringBuffer(markupFileClass.getName());
@@ -92,7 +94,8 @@
 				{
 					if (tag.getAttributes().get("id") == null)
 					{
-						tag.getAttributes().put("id", headElementIdPrefix + nextValue());
+						((TagAttributes)tag.getAttributes()).putInternal("id", headElementIdPrefix +
+							nextValue());
 						tag.setModified(true);
 					}
 				}