-
Notifications
You must be signed in to change notification settings - Fork 566
Update Dependency: commons-fileupload2-jakarta-servlet6 to 2.0.0-M4 #1465
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
Conversation
This is to update dependency on commons-fileupload2-core from 2.0.0-M2 to 2.0.0-M4
...ore/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java
Outdated
Show resolved
Hide resolved
…/serverless/proxy/internal/servlet/AwsHttpServletRequest.java
addPart(multipartFormParameters, item.getFieldName(), newPart); | ||
} catch (IOException e) { | ||
log.error("Encounter issue adding form multipart", e); | ||
throw new RuntimeException(e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see https://javadoc.io/static/jakarta.servlet/jakarta.servlet-api/6.0.0/jakarta.servlet/jakarta/servlet/http/HttpServletRequest.html#getPart(java.lang.String) and https://javadoc.io/doc/jakarta.servlet/jakarta.servlet-api/6.0.0/jakarta.servlet/jakarta/servlet/http/HttpServletRequest.html:
IOException - if an I/O error occurred during the retrieval of the Part components of this request
ServletException - if this request is not of type multipart/form-data
IllegalStateException - if the request body is larger than maxRequestSize, or any Part in the request is larger than maxFileSize, or there is no @MultipartConfig or multipart-config in deployment descriptors
Therefore I wouldn't make it a generic RuntimeException
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Thank you. Since getMultipartFormParametersMap()
used in getParts()
which throw IOException, ServletException in its signature, i updated the code to just throw IOException
from getMultipartFormParametersMap()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. You can even remove the try/ catch block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course.
AwsProxyRequestPart newPart = new AwsProxyRequestPart(item.get()); | ||
newPart.setName(item.getFieldName()); | ||
newPart.setSubmittedFileName(fileName); | ||
newPart.setContentType(item.getContentType()); | ||
newPart.setSize(item.getSize()); | ||
item.getHeaders().getHeaderNames().forEachRemaining(h -> { | ||
newPart.addHeader(h, item.getHeaders().getHeader(h)); | ||
}); | ||
addPart(multipartFormParameters, item.getFieldName(), newPart); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Go back to original indentation here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. It was not highlighted locally. Removing extra space.
This is to update dependency on commons-fileupload2-core from 2.0.0-M2 to 2.0.0-M4
By submitting this pull request