You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@struts.apache.org by cr...@apache.org on 2005/11/15 02:53:45 UTC

svn commit: r344288 - in /struts/shale/trunk/core-library/src: java/org/apache/shale/view/impl/DefaultViewControllerMapper.java test/org/apache/shale/view/impl/DefaultViewControllerMapperTestCase.java

Author: craigmcc
Date: Mon Nov 14 17:53:40 2005
New Revision: 344288

URL: http://svn.apache.org/viewcvs?rev=344288&view=rev
Log:
[37477] Prefix directory names starting with a digit

Prefix the returned value for paths whose initial character after the
leading slash is a digit, in addition to prefixing reserved words.  This
avoids cases where the returned value is evaluated as a variable name,
because variable names cannot start with a digit.

Modified:
    struts/shale/trunk/core-library/src/java/org/apache/shale/view/impl/DefaultViewControllerMapper.java
    struts/shale/trunk/core-library/src/test/org/apache/shale/view/impl/DefaultViewControllerMapperTestCase.java

Modified: struts/shale/trunk/core-library/src/java/org/apache/shale/view/impl/DefaultViewControllerMapper.java
URL: http://svn.apache.org/viewcvs/struts/shale/trunk/core-library/src/java/org/apache/shale/view/impl/DefaultViewControllerMapper.java?rev=344288&r1=344287&r2=344288&view=diff
==============================================================================
--- struts/shale/trunk/core-library/src/java/org/apache/shale/view/impl/DefaultViewControllerMapper.java (original)
+++ struts/shale/trunk/core-library/src/java/org/apache/shale/view/impl/DefaultViewControllerMapper.java Mon Nov 14 17:53:40 2005
@@ -32,6 +32,11 @@
  * <li>If the resulting name matches one of the reserved names recognized
  *     by the default <code>VariableResolver</code>, prefix it with an
  *     underscore character ("_"), to avoid problems loading managed beans.</li>
+ * <li>If the resulting name starts with a digit character, prefix it with
+ *     an underscore character("_"), to avoid problems evaluating value
+ *     binding expressions using it (because this would be treated as a
+ *     variable name starting with a digit, and that is not allowed by the
+ *     syntax of expression evaluation).</li>
  * </ul>
  *
  * <p>Examples of correct managed bean names for typical JSF view identifiers,
@@ -97,6 +102,8 @@
         }
         viewId = viewId.replace('/', '$');
         if (reserved.contains(viewId)) {
+            return "_" + viewId;
+        } else if ((viewId.length() > 0) && Character.isDigit(viewId.charAt(0))) {
             return "_" + viewId;
         } else {
             return viewId;

Modified: struts/shale/trunk/core-library/src/test/org/apache/shale/view/impl/DefaultViewControllerMapperTestCase.java
URL: http://svn.apache.org/viewcvs/struts/shale/trunk/core-library/src/test/org/apache/shale/view/impl/DefaultViewControllerMapperTestCase.java?rev=344288&r1=344287&r2=344288&view=diff
==============================================================================
--- struts/shale/trunk/core-library/src/test/org/apache/shale/view/impl/DefaultViewControllerMapperTestCase.java (original)
+++ struts/shale/trunk/core-library/src/test/org/apache/shale/view/impl/DefaultViewControllerMapperTestCase.java Mon Nov 14 17:53:40 2005
@@ -92,6 +92,20 @@
     }
 
 
+    // Test numeric prefix in logical name
+    public void testNumeric() {
+
+        // Positive ... should be prefixed
+        assertEquals("_1$a", dvcm.mapViewId("/1/a.jsp"));
+        assertEquals("_1$1$a", dvcm.mapViewId("/1/1/a.jsp"));
+
+        // Negative ... should not be prefixed
+        assertEquals("a$1$b", dvcm.mapViewId("/a/1/b.jsp"));
+
+    }
+
+
+
     // Test a pristine instance
     public void testPristine() {
 



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


Re: svn commit: r344288 - in /struts/shale/trunk/core-library/src: java/org/apache/shale/view/impl/DefaultViewControllerMapper.java test/org/apache/shale/view/impl/DefaultViewControllerMapperTestCase.java

Posted by Rahul Akolkar <ra...@gmail.com>.
On 11/15/05, Craig McClanahan <cr...@apache.org> wrote:
> On 11/14/05, Rahul Akolkar <ra...@gmail.com> wrote:
<snip/>
> > Best practices such as:
> > * Not having a view placed at context root with just digits in the
> > filename
>
>
> Doesn't that use case get covered by the patch we just committed? If the
> input path is "/123.jsp", then this will now get translated to "_123"
> instead of "123" like it used to.
>
<snip/>

Sure does.

>
> Yes, I think the class Javadocs for DVCM is the right place for such
> information (since the restrictions are based on the algorithm embedded in
> this particular class). Please feel free to draft a patch so that we can
> cover this kind of thing.
>
<snap/>

Will add to the coming attractions.

-Rahul

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


Re: svn commit: r344288 - in /struts/shale/trunk/core-library/src: java/org/apache/shale/view/impl/DefaultViewControllerMapper.java test/org/apache/shale/view/impl/DefaultViewControllerMapperTestCase.java

Posted by Craig McClanahan <cr...@apache.org>.
On 11/14/05, Rahul Akolkar <ra...@gmail.com> wrote:
>
> On 11/14/05, craigmcc@apache.org <cr...@apache.org> wrote:
> > Author: craigmcc
> > Date: Mon Nov 14 17:53:40 2005
> > New Revision: 344288
> >
> > URL: http://svn.apache.org/viewcvs?rev=344288&view=rev
> > Log:
> > [37477] Prefix directory names starting with a digit
> >
> <snip/>
> > viewId = viewId.replace('/', '$');
> > if (reserved.contains(viewId)) {
> > + return "_" + viewId;
> > + } else if ((viewId.length() > 0) && Character.isDigit(viewId.charAt(0)))
> {
> > return "_" + viewId;
> <snap/>
>
> Craig,
>
> I had thought of something else, and the first part of the compound
> condition above brought it back to me.
>
> The DefaultViewControllerMapper algorithm is beautiful in its
> simplicity, but also imposes a few restrictions on the naming of
> views. I understand that the ViewControllerMapper can be easily
> replaced, but I suspect many of us will be using the default impl.
>
> Best practices such as:
> * Not having a view placed at context root with just digits in the
> filename


Doesn't that use case get covered by the patch we just committed? If the
input path is "/123.jsp", then this will now get translated to "_123"
instead of "123" like it used to.

* Not having a hyphen in the filename
> might serve as a nice heads-up.


A more general description of the restriction (with this as an example) is
that the resulting view identifier needs to be a valid identifier, as
defined by the syntax of the expression language.

I can draft a patch for Javadoc, is
> that where you'd post those?


Yes, I think the class Javadocs for DVCM is the right place for such
information (since the restrictions are based on the algorithm embedded in
this particular class). Please feel free to draft a patch so that we can
cover this kind of thing.

-Rahul


Craig

Re: svn commit: r344288 - in /struts/shale/trunk/core-library/src: java/org/apache/shale/view/impl/DefaultViewControllerMapper.java test/org/apache/shale/view/impl/DefaultViewControllerMapperTestCase.java

Posted by Rahul Akolkar <ra...@gmail.com>.
On 11/14/05, craigmcc@apache.org <cr...@apache.org> wrote:
> Author: craigmcc
> Date: Mon Nov 14 17:53:40 2005
> New Revision: 344288
>
> URL: http://svn.apache.org/viewcvs?rev=344288&view=rev
> Log:
> [37477] Prefix directory names starting with a digit
>
<snip/>
>         viewId = viewId.replace('/', '$');
>         if (reserved.contains(viewId)) {
> +            return "_" + viewId;
> +        } else if ((viewId.length() > 0) && Character.isDigit(viewId.charAt(0))) {
>             return "_" + viewId;
<snap/>

Craig,

I had thought of something else, and the first part of the compound
condition above brought it back to me.

The DefaultViewControllerMapper algorithm is beautiful in its
simplicity, but also imposes a few restrictions on the naming of
views. I understand that the ViewControllerMapper can be easily
replaced, but I suspect many of us will be using the default impl.

Best practices such as:
 * Not having a view placed at context root with just digits in the filename
 * Not having a hyphen in the filename
might serve as a nice heads-up. I can draft a patch for Javadoc, is
that where you'd post those?

-Rahul

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


Re: svn commit: r344288 - in /struts/shale/trunk/core-library/src: java/org/apache/shale/view/impl/DefaultViewControllerMapper.java test/org/apache/shale/view/impl/DefaultViewControllerMapperTestCase.java

Posted by Craig McClanahan <cr...@apache.org>.
Oops, forgot to give credit to Rahul Akolkar (rahul AT
apache.org<http://apache.org>)
for the proposed patch in the SVN log.

Craig


On 11/14/05, craigmcc@apache.org <cr...@apache.org> wrote:
>
> Author: craigmcc
> Date: Mon Nov 14 17:53:40 2005
> New Revision: 344288
>
> URL: http://svn.apache.org/viewcvs?rev=344288&view=rev
> Log:
> [37477] Prefix directory names starting with a digit
>
> Prefix the returned value for paths whose initial character after the
> leading slash is a digit, in addition to prefixing reserved words. This
> avoids cases where the returned value is evaluated as a variable name,
> because variable names cannot start with a digit.
>
> Modified:
>
> struts/shale/trunk/core-library/src/java/org/apache/shale/view/impl/DefaultViewControllerMapper.java
>
> struts/shale/trunk/core-library/src/test/org/apache/shale/view/impl/DefaultViewControllerMapperTestCase.java
>
> Modified:
> struts/shale/trunk/core-library/src/java/org/apache/shale/view/impl/DefaultViewControllerMapper.java
> URL:
> http://svn.apache.org/viewcvs/struts/shale/trunk/core-library/src/java/org/apache/shale/view/impl/DefaultViewControllerMapper.java?rev=344288&r1=344287&r2=344288&view=diff
>
> ==============================================================================
> ---
> struts/shale/trunk/core-library/src/java/org/apache/shale/view/impl/DefaultViewControllerMapper.java
> (original)
> +++
> struts/shale/trunk/core-library/src/java/org/apache/shale/view/impl/DefaultViewControllerMapper.java
> Mon Nov 14 17:53:40 2005
> @@ -32,6 +32,11 @@
> * <li>If the resulting name matches one of the reserved names recognized
> * by the default <code>VariableResolver</code>, prefix it with an
> * underscore character ("_"), to avoid problems loading managed
> beans.</li>
> + * <li>If the resulting name starts with a digit character, prefix it
> with
> + * an underscore character("_"), to avoid problems evaluating value
> + * binding expressions using it (because this would be treated as a
> + * variable name starting with a digit, and that is not allowed by the
> + * syntax of expression evaluation).</li>
> * </ul>
> *
> * <p>Examples of correct managed bean names for typical JSF view
> identifiers,
> @@ -97,6 +102,8 @@
> }
> viewId = viewId.replace('/', '$');
> if (reserved.contains(viewId)) {
> + return "_" + viewId;
> + } else if ((viewId.length() > 0) && Character.isDigit(viewId.charAt(0)))
> {
> return "_" + viewId;
> } else {
> return viewId;
>
> Modified:
> struts/shale/trunk/core-library/src/test/org/apache/shale/view/impl/DefaultViewControllerMapperTestCase.java
> URL:
> http://svn.apache.org/viewcvs/struts/shale/trunk/core-library/src/test/org/apache/shale/view/impl/DefaultViewControllerMapperTestCase.java?rev=344288&r1=344287&r2=344288&view=diff
>
> ==============================================================================
> ---
> struts/shale/trunk/core-library/src/test/org/apache/shale/view/impl/DefaultViewControllerMapperTestCase.java
> (original)
> +++
> struts/shale/trunk/core-library/src/test/org/apache/shale/view/impl/DefaultViewControllerMapperTestCase.java
> Mon Nov 14 17:53:40 2005
> @@ -92,6 +92,20 @@
> }
>
>
> + // Test numeric prefix in logical name
> + public void testNumeric() {
> +
> + // Positive ... should be prefixed
> + assertEquals("_1$a", dvcm.mapViewId("/1/a.jsp"));
> + assertEquals("_1$1$a", dvcm.mapViewId("/1/1/a.jsp"));
> +
> + // Negative ... should not be prefixed
> + assertEquals("a$1$b", dvcm.mapViewId("/a/1/b.jsp"));
> +
> + }
> +
> +
> +
> // Test a pristine instance
> public void testPristine() {
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
>