Skip to content

PR to fix #824 and reference to #823 #828

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
May 27, 2024
Merged

PR to fix #824 and reference to #823 #828

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0386367
Updated DefaultEncoder.getCanonicalizedURI(URI) javadoc to indicate t…
xeno6696 Jan 20, 2024
ddd4a3a
Per issue #824. Updated DefaultEncoder.getCanonicalizedURI(URI) java…
xeno6696 Jan 20, 2024
7c4f232
Merge branch 'develop' of https://github.com/xeno6696/esapi-java-lega…
xeno6696 Jan 22, 2024
3807f52
Fixed #824 by nesting the original canonicalize call into the else bl…
xeno6696 Jan 23, 2024
580ccbc
Merge branch 'develop' into develop
xeno6696 Jan 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Updated DefaultEncoder.getCanonicalizedURI(URI) javadoc to indicate t… 
…hat the method takes into consideration canonicalization of mixed/multi encoded URLs as specified in ESAPI.props 'allowMixed' and 'allowMultiple' accordingly.
  • Loading branch information
@xeno6696
xeno6696 committed Jan 20, 2024
commit 03863678c734f08d40778a7aaf93023705bf53f1
3 changes: 3 additions & 0 deletions src/main/java/org/owasp/esapi/reference/DefaultEncoder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,9 @@ public byte[] decodeFromBase64(String input) throws IOException {
* This will extract each piece of a URI according to parse zone as specified in <a href="https://www.ietf.org/rfc/rfc3986.txt">RFC-3986</a> section 3,
* and it will construct a canonicalized String representing a version of the URI that is safe to
* run regex against.
*
* NOTE: This method will obey the ESAPI.properties configurations for allowing
* Mixed and Multiple Encoding URLs.
Copy link
Contributor

@kwwall kwwall Feb 24, 2024

Choose a reason for hiding this comment

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

Was that not the intent and expectation all along? If not, they this will result in unexpected behavior because I think that is not intuitive and therefore it should probably be a WARNING rather than just a NOTE.

OTOH, if this was the intended behavior all along (which I think is the case), then I would contend that this NOTE is probably not needed because it is likely to be confusing to those reading the Javadoc because all you are really saying is "Hey, this now works like it was originally intended for relative URIs but it didn't before because of a bug that was present. See GitHub issues #823 and #824." So, I don't think we really need this comment in the Javadoc. I think it will cause confusion and cause people to ask "why is this here; isn't that obvious?".

Therefore, my recommendation is either delete it, or if you want to keep it change it to a non-Javadoc comment and reference the issues.

Make take on this

*
* @param dirtyUri
* @return Canonicalized URI string.
Expand Down