kkolinko commented on PR #681: URL: https://github.com/apache/tomcat/pull/681#issuecomment-1864808958
1. There are case-insensitive file systems out there... I wonder whether those default extensions should be treated case-insensitively. (If one is serving a web site from an USB stick or a memory card formatted with FAT? From a CD Drive? It is possible, but rare nowadays.) 2. Add "*.mjs" to the list (see https://bz.apache.org/bugzilla/show_bug.cgi?id=68378 ) 3. Documentation: The value in "The default is ..." does not match the actual value of DEFAULT_NO_NONCE_URL_PATTERNS; 4. Documentation: "Complete regular expression ... Note that patterns cannot contain a comma" I think if the value starts and ends with a '/'. it would be better to treat it whole as a single RegExp. Commas are useful in RegExes and disallowing them in this case does not look like a benefit. 5. protected boolean skipNonceCheck(HttpServletRequest request) { It is hard-coded to look for GET. How about a HEAD request? 6. protected boolean skipNonceCheck(HttpServletRequest request) { Further in that method. "if (!entryPoints.contains(requestedPath)) { return false; }" - note that unless it is an entry point, processing will end here and subsequent lines will not run. I think it was intended to be the opposite. 7. private boolean shouldAddNonce(String url) { ... } I think that it would make sense to skip adding nonces to the entryPoints. (As a use case: the front page of Manager web application). 8. It would be good to have some test cases. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
