Initial 2.5.2.0 release preparation #784
Conversation
Updated date.prev_release to compute CHANGELOG for 'mvn site'.
…auses failure in running 'mvn site'.
…ies.FileUploadAllowAnonymousUser) to address CVE-2023-24998.
…ich cause problems with 'mvn site'.
…adFileCount and HttpUtilities.FileUploadAllowAnonymousUser.
…ation (as much as possible).
|
@jeremiahjstacey and @xeno6696 - One of you at least needs to review and approve this PR. (I can do the merge, as I want to add a few comments during the merge.) Jeremiah, please at least look at HTTPUtilitiesTest.java and the changes there. You will see that I have a few questions and you seem to have a lot of expertise about JUnit. There is a chuck of new code in DefaultHTTPUtilities.java references FIXME and mentions your GitHub ID. It wasn't able to see the log output when I locally changed the log level from INFO to DEBUG. Maybe you can tell me what I was doing wrong. I changed it both places in file esapi-java-logging.properties which I thought was the only place I needed to tweak it. |
| try { | ||
| response2 = ESAPI.httpUtilities().getFileUploads(request2, home); | ||
| } catch( ValidationUploadException vuex ) { | ||
| caughtExpectedException = true; |
There was a problem hiding this comment.
There are built-in ways to assert that a test fails as expected using content like the ExpectedException rule (junit4) or using @Test (expected=ValidationUploadException.class)
Just fyi points that achieve the same general goal.
There are possible optimizations for this test, but I believe that overall it achieves the desired effect.
There was a problem hiding this comment.
Good point. I will fix that. I probably need to import the appropriate annotation though as well too. And I will also add @ignore annotation instead of commenting everything out from that one test like I did which is just ugly.
There was a problem hiding this comment.
The @ignore annotation didn't work, probably because we are still using the JUnit 3 stype TestSuite test setup, so I just left the testGetFileUploadsUnauthenticatedUser commented out.
There was a problem hiding this comment.
My apologies for not returning to this one in time, I can add refactoring this test to the percent codec update for UTF-8 on my TODO. This one will probably be done sooner.
src/main/java/org/owasp/esapi/reference/DefaultHTTPUtilities.java
Outdated
Show resolved
Hide resolved
|
@jeremiahjstacey - In case @xeno6696 doesn't get around to approving this, let me do the merge as I may wish to add some additional comments here. (Or not; haven't really decided, but probably will want to edit them a bit and prefer not to have to do 'git comment --amend'.) So I will wait until COB tomorrow for @xeno6696 approval, but if he doesn't have time, I will proceed with the merge without him. P.S.- Thanks for the review and the tips! (I was going to write 'pointers', but this is Java, so no pointers.) |
| "--ridiculous\r\n" + | ||
| "Content-Disposition: form-data; name=\"full-name\"\r\n\r\n" + | ||
| "kevin w wall\r\n" + | ||
| "--ridiculous\r\n" + |
There was a problem hiding this comment.
I hear a song in my head with all these calls of "rediculous..." ;-)
| response2.forEach(file -> file.delete()); | ||
| } | ||
| // If this assertion fails, check the property HttpUtilities.MaxUploadFileCount in | ||
| // 'src/test/resources/esapi/ESAPI.properties' to make sure it is still to 2. |
There was a problem hiding this comment.
Thanks for being explicit enough for newbies here.
There was a problem hiding this comment.
Newbies? Heck, I wrote that comment for myself so I wouldn't have to go out and debug the code in a year or so. :)
Mostly to address CVE-2023-24998 in Apache Commons Files Upload, but other minor changes described in the 2.5.2.0 release notes (which are included).