Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

URI compliance modes for #6001 #6006

Merged
merged 8 commits into from Mar 2, 2021
Merged

Conversation

gregw
Copy link
Contributor

@gregw gregw commented Feb 24, 2021

Refactor the HttpCompliance changes of #6003 and #6002 for #6001 to a new UriCompliance class, changing default to SAFE

gregw and others added 4 commits February 24, 2021 19:17
…parators

default modes allows both ambiguous separators and segments, but still forbids ambiguous parameters
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
…-ambiguous-uris

Fix #6001 separate compliance modes for ambiguous URI segments, params and separators
Fixed checkstyle
@gregw gregw changed the title Merge of #6001 uri compliance to jetty-10 URI compliance modes for #6001 Feb 25, 2021
Improve names
Improved javadoc
@gregw
Copy link
Contributor Author

gregw commented Feb 25, 2021

@sbordet I've updated the names and the javadoc

updates after review
@gregw gregw requested a review from sbordet February 25, 2021 17:27
Copy link
Contributor

@sbordet sbordet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, apart the small niggle about EnumSet usage.

private static final AtomicInteger __custom = new AtomicInteger();

public static UriCompliance valueOf(String name)
{
for (UriCompliance compliance : KNOWN_MODES)
for (UriCompliance compliance : Arrays.asList(DEFAULT, LEGACY, RFC3986, STRICT, SAFE))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not using EnumSet.allOf() or similar?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because they are not enums! They are just static final fields.
They were enums in 9, but that meant users could not define their own modes, so we have the CUSTOM[0-4] hack there.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doh! Ship it.

Copy link
Contributor

@sbordet sbordet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM apart EnumSet usage.

@gregw gregw merged commit 06e1a7e into jetty-10.0.x Mar 2, 2021
@joakime joakime deleted the jetty-10.0.x-6001-UriCompliance branch March 3, 2021 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants