EZP-30956: fix false positive view parameters detection

[ITernovtsii]

Question	Answer
JIRA issue	EZP-30956
Bug/Improvement	yes
New feature	no
Target version	6.13 / 7.x
BC breaks	no
Tests pass	yes
Doc needed	no

Replacement of #2771

TODO:

• Implement feature / fix a bug.
• Implement tests.
• Fix new code according to Coding Standards ($ composer fix-cs).
• Ask for Code Review.

[ITernovtsii]

IMHO, it will be good to remove named parameters implementation starting from eZPlatform v3

[glye]

Good addition. Could perhaps be even more rigid? View parameters are defined as
/(param_name)/param_value. With a regexp we can easily detect that both the name and the value are present. But I don't know if we have a clear definition of what characters are allowed in names and values...

[gggeek]

@glye iirc in ez-legacy the view parameters were in fact more flexible than that - even though this is the way they were used in 100% of the apps and extensions that I ever came across.
Not sure if that is important in the context of eZPlatform...

[ITernovtsii]

Good addition. Could perhaps be even more rigid? View parameters are defined as
/(param_name)/param_value. With a regexp we can easily detect that both the name and the value are present. But I don't know if we have a clear definition of what characters are allowed in names and values...

Agree, the whole implementation of named parameters will look better with proper regexp.
But, this bug is 7 years old and named parameters might be removed in v3.
So, I decided to just fix download file issue here, and keep named parameters work as they are now.

[glye]

As discussed in #2771 I support this fix, though I don't think it is sufficient in itself, for stable future functionality. But if named parameters are indeed removed in 3.x, then it may be good enough. In any case it solves the immediate problem, while we think about dealing with the underlying cause.

NB: As a bug fix this should be PR-ed against 6.7 or 6.13, depending on whether it passes review before or after new years. Well, let's say 6.13. A new 6.7 release is unlikely now.

[ITernovtsii]

Hi @glye, rebased to 6.13, thanks.

[glye]

Thanks! I just realised you could break this with a URL like /foo)/(bar/, but that would break before this fix as well, so not a reason not to accept the PR. For this reason and others I mentioned before I don't think the issue is resolved, but the PR is a good improvement and can IMO be merged.

[alongosz]

I have a weird feeling that we're reinventing a wheel here, but no other idea now, so +1