coheigea commented on PR #3572: URL: https://github.com/apache/cxf/pull/3572#issuecomment-6146309864
Thanks, this is much better and covers everything I raised: the parser is untouched, the existing stripPath is reused, and it’s applied at every place that reads the filename. A couple of small things before merge: safeFileName returns null for any name containing :, so a valid Unix or macOS name like Meeting 10:30.pdf loses its filename entirely. Would you consider replacing : (and NUL) with _ instead of rejecting? I’m fine either way if you’d rather keep it strict. In EntityPartUtils and EntityPartProvider, whether a part is a file is decided by !StringUtils.isEmpty(fileName). That now uses the cleaned name, so filename="reports/" or filename=".." turns a file part into a plain field. Could the file check use the raw cd.getFilename() != null, with safeFileName used only for the name itself? Could you add a small test in rt/frontend/jaxrs for the EntityPart paths? The current tests only cover core. Optional: a javadoc note on ContentDisposition.getFilename() saying the value is untrusted and pointing to FileUtils.safeFileName. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
