The regex replacement strips %blah and replaces it with *. While, the search pattern knows that menu replacement arguments end with a /, the replacement pattern does not. The change is simple, use '[^\/]*' as the replacement string instead of '*'.

I noticed this problem because I added a modal to 'user/%user' and the regex started turning anything user/ into a modal, for example user/logout.

Files: 
CommentFileSizeAuthor
#12 ctools_automodal-regex_too_permissive-1439762-12.patch3.33 KBacrollet
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#10 ctools_automodal-regex_too_permissive-1439762-10.patch3.14 KBacrollet
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch ctools_automodal-regex_too_permissive-1439762-10.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#8 1439762-8_regex_too_permissive.patch2.65 KBjlyon
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#6 1439762-6_regex_too_permissive.patch2.42 KBwojtha
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#1 1439762_1.patch2.1 KBchx
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
regex.patch522 bytesdouggreen
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Comments

StatusFileSize
new2.1 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Aye but that patch is waaaaaay too little. My patch is PHP 5.3 anyone is welcome to convert down to 5.2.

I need comments. Because I'm a dumb maintainer.

Status:Needs review» Needs work

I'm seeing these when installing an install profile, likely just needs an empty() check for when no paths are modal.

preg_match(): Empty regular expression ctools_automodal.module:53    [warning]
preg_match(): Empty regular expression ctools_automodal.module:53    [warning]
implode(): Invalid arguments passed ctools_automodal.module:42       [warning]

@catch Unrelated error or error that happens with either of the patches applied?

Just the latest patch.

Status:Needs work» Needs review
StatusFileSize
new2.42 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Trying to fix the bugs reported by catch in #3

This will throw a fatal error on php < 5.3 because of the line:

$convert_to_regexp = function ($value) { return preg_replace('/%[^\/]*/', '[^/]+', $value); };.

Any possible work-arounds? See https://github.com/wojtha/ctools_automodal/issues/1.

Edit: @chx: Obviously you knew that it was php 5.3 because you mentioned it in your post. Sorry for not catching it. I think it is important to make it work in php 5.2 for the issues outlined in the github issue. I'll see if I can figure it out.

StatusFileSize
new2.65 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

This update to #6 makes the patch work with php < 5.3.

Just FYI, the last bugfix release of 5.2 was two years ago. The last security update was 1.5 years ago. I know, I know. But, still.

StatusFileSize
new3.14 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch ctools_automodal-regex_too_permissive-1439762-10.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

re-roll of #8 attached. This makes 2 changes:

1) The not-modal regexp was too greedy when there were no paths in it (@^@)
2) Adds comments per #2

Status:Needs review» Needs work

The last submitted patch, ctools_automodal-regex_too_permissive-1439762-10.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new3.33 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

updated patch attached, applies properly against head and eliminates a notice.