neharob / prettyfaces

Automatically exported from code.google.com/p/prettyfaces
0 stars 0 forks source link

Using curly quantifiers in custom-regexes causes parsing failure #94

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
This test fails but should pass.

   @Test
   public void testNoLeadingSlashWithSingleParameter() throws Exception
   {
      URLPatternParser parser = new URLPatternParser("#{ /faqs(/indice/[0-9]{1,2})?/ menuController.faqsindice}/");
      List<PathParameter> params = parser.parse(new URL("faqs/"));
   }

Original issue reported on code.google.com by lincolnb...@gmail.com on 1 Mar 2011 at 6:01

GoogleCodeExporter commented 9 years ago
are you sure? afaik we never supported regexes going over / 

Original comment by dominik....@gmail.com on 1 Mar 2011 at 6:08

GoogleCodeExporter commented 9 years ago
Yeah, we support that so that people can do things like custom segments:

/one/two/three/.../infinity

Original comment by lincolnb...@gmail.com on 1 Mar 2011 at 6:20

GoogleCodeExporter commented 9 years ago
Hey Lincoln,

I just looked into this issue. I think the fix is very simple. But I want to 
get your opinion on this.

I think its caused by the RegexOverride class. This class contains the 
following pattern for matching path parameters containing custom regular 
expressions:

  public static final String REGEX = "(\\#\\{)" + "(\\s*/(.*?)/)" + "\\s*([^}/]*\\})";

The regex pattern is matched via (.*?) which is a "reluctant quantifier". The 
test passes fine if I change it to (.*) which is a "greedy quantifier" 
resulting in:

  public static final String REGEX = "(\\#\\{)" + "(\\s*/(.*)/)" + "\\s*([^}/]*\\})";

Your failing test case and all others are now running fine.

What do you think? You are much better at regular expressions than I am! Does 
it make sense? :)

Christian

Original comment by chkalt on 2 Oct 2011 at 2:38

GoogleCodeExporter commented 9 years ago
Attached the patch!

Original comment by chkalt on 2 Oct 2011 at 2:42

Attachments:

GoogleCodeExporter commented 9 years ago
Hmm, TBH, I think the whole parsing system in PrettyFaces needs to be replaced 
with the one from Rewrite. 

Take a look at ParameterizedPattern, which is a new class I wrote that uses 
proper tokenized parsing.

Your fix might work, but try what happens if you use more than one parameter. 
If this works, then I think it should be fairly safe to apply:

   @Test
   public void testNoLeadingSlashWithSingleParameter() throws Exception
   {
      URLPatternParser parser = new URLPatternParser("#{ /faqs(/indice/[0-9]{1,2})?/ menuController.faqsindice}/#{anotherParam}");
      List<PathParameter> params = parser.parse(new URL("faqs/"));
      assertEquals(2, params.size());
   }

Original comment by lincolnb...@gmail.com on 2 Oct 2011 at 6:22

GoogleCodeExporter commented 9 years ago
Yeah, you are completely right. My fix don't work for more than one parameter. 
I didn't thought about that. Too bad! :)

You are right! We should think about the parsing system. I'll take a look at 
ParameterizedPattern to check how you do it there..

Original comment by chkalt on 3 Oct 2011 at 10:12

GoogleCodeExporter commented 9 years ago
We have moved the project to GitHub.

If this issue is still relevant, please reopen it here:

https://github.com/ocpsoft/prettyfaces/issues

Original comment by chkalt on 7 Mar 2012 at 6:15