[GitHub] struts pull request #157: WW-4834 fixed faulty regex

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] struts pull request #157: WW-4834 fixed faulty regex

cnenning
GitHub user sdutry opened a pull request:

    https://github.com/apache/struts/pull/157

    WW-4834 fixed faulty regex

   

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/sdutry/struts WW-4834

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/struts/pull/157.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #157
   
----
commit 8a04e80f01350c90f053d71366d5e0c2186fded5
Author: Stefaan Dutry <[hidden email]>
Date:   2017-08-02T07:31:11Z

    WW-4834 fixed faulty regex

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] struts issue #157: WW-4834 fixed faulty regex

cnenning
Github user lukaszlenart commented on the issue:

    https://github.com/apache/struts/pull/157
 
    Could you wait a bit with merging this PR? I have asked somebody to take a look on this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] struts issue #157: WW-4834 fixed faulty regex

cnenning
In reply to this post by cnenning
Github user sdutry commented on the issue:

    https://github.com/apache/struts/pull/157
 
    Ok,
   
    I will wait.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] struts issue #157: WW-4834 fixed faulty regex

cnenning
In reply to this post by cnenning
Github user atcazzual commented on the issue:

    https://github.com/apache/struts/pull/157
 
    The expression did not seem to work at all until I escaped the slashes, changing  `/`  to  `\/`
   
    Once I got it working, there then seems to be a bug in the new expression when matching on URLs that use IP addresses.   The grouping has changed causing two problems with matching IP addresses.
   
    1. The dot `.` character that delimits the octets in an IP address only applies to the last condition, `25[0-5]\.` on line 57, instead of all conditions for an IP octet. This makes matching most IP address fail.   The only IPs that will match would need to have 3-digit octets for the first three where the first two-digits are `25`.  _NOTE: This seems to have been resolved by the commit above._
    2. The conditions for the last octet are no longer grouped (line 58) making the OR `|` operator act on what was a higher level group.  Because of this, the fourth octet would have to be only one or two digits.
   
    For example, only IPs like the following will pass validation:
    http://**25**3.**25**4.**25**5.1  (mostly resolved by the commit above)
    http://**25**3.**25**4.**25**5.12 (mostly resolved by the commit above)
   
    After the commit above, any IP with 3 digits in the last octet will **not** pass validation:
    http<nolink>://1.2.3.**100**
    http<nolink>://1.2.3.**255**


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] struts issue #157: WW-4834 fixed faulty regex

cnenning
In reply to this post by cnenning
Github user sdutry commented on the issue:

    https://github.com/apache/struts/pull/157
 
    > After the commit above, any IP with 3 digits in the last octet will not pass validation
   
    You are right, i forgot the grouping (meaning the or statements mean something completely different), i'll fix this ASAP
   
    > The expression did not seem to work at all until I escaped the slashes, changing / to \/
    I don't see why this would be. I am not aware that a "/" is a special sign inside a java string or java RegEx. Or is it because this same regex is used in javascript regex validation (where the '/' sign does mean something). I'll change it back ASAP too


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] struts issue #157: WW-4834 fixed faulty regex

cnenning
In reply to this post by cnenning
Github user lukaszlenart commented on the issue:

    https://github.com/apache/struts/pull/157
 
    I think we are good here, thanks a lot for yours work :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] struts issue #157: WW-4834 fixed faulty regex

cnenning
In reply to this post by cnenning
Github user sdutry commented on the issue:

    https://github.com/apache/struts/pull/157
 
    @lukaszlenart
    Sorry for breaking it in the first place. That wasn't my intention.
   
    Do you want me to merge this now, or am i still overlooking stuff?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] struts issue #157: WW-4834 fixed faulty regex

cnenning
In reply to this post by cnenning
Github user lukaszlenart commented on the issue:

    https://github.com/apache/struts/pull/157
 
    I would wait a bit and give Adam a chance to post a comment (if he wants to). No rush :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] struts issue #157: WW-4834 fixed faulty regex

cnenning
In reply to this post by cnenning
Github user atcazzual commented on the issue:

    https://github.com/apache/struts/pull/157
 
    Looks good to me.  All matching capabilities from prior to the optimizations seem to be there.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] struts pull request #157: WW-4834 fixed faulty regex

cnenning
In reply to this post by cnenning
Github user asfgit closed the pull request at:

    https://github.com/apache/struts/pull/157


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Loading...