Skip to content
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

Jump to bottom

GH-4374: Eliminate commons-fileupload dependency #4375

Merged
merged 1 commit into from
Mar 4, 2023
Merged

GH-4374: Eliminate commons-fileupload dependency #4375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 0 additions & 5 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,11 +391,6 @@
<artifactId>commons-cli</artifactId>
<version>1.4</version>
</dependency>
<dependency>
<groupId>commons-fileupload</groupId>
<artifactId>commons-fileupload</artifactId>
<version>1.3.3</version>
</dependency>
<dependency>
<groupId>commons-io</groupId>
<artifactId>commons-io</artifactId>
Expand Down
5 changes: 0 additions & 5 deletions tools/workbench/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,6 @@
<artifactId>javax.servlet-api</artifactId>
<scope>provided</scope>
</dependency>
<!-- Required for CommonsMultipartResolver from the Spring Framework -->
<dependency>
<groupId>commons-fileupload</groupId>
<artifactId>commons-fileupload</artifactId>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
Expand Down
3 changes: 1 addition & 2 deletions tools/workbench/src/main/java/org/eclipse/rdf4j/workbench/commands/AddServlet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

import javax.servlet.http.HttpServletResponse;

import org.apache.commons.fileupload.FileUploadException;
import org.eclipse.rdf4j.model.Resource;
import org.eclipse.rdf4j.query.QueryResultHandlerException;
import org.eclipse.rdf4j.repository.RepositoryConnection;
Expand All @@ -41,7 +40,7 @@ public class AddServlet extends TransformationServlet {

@Override
protected void doPost(WorkbenchRequest req, HttpServletResponse resp, String xslPath)
throws IOException, RepositoryException, FileUploadException, QueryResultHandlerException {
throws IOException, RepositoryException, QueryResultHandlerException {
try {
String baseURI = req.getParameter("baseURI");
String contentType = req.getParameter("Content-Type");
Expand Down
2 changes: 2 additions & 0 deletions tools/workbench/src/main/java/org/eclipse/rdf4j/workbench/proxy/WorkbenchGateway.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import javax.servlet.ServletConfig;
import javax.servlet.ServletException;
import javax.servlet.annotation.MultipartConfig;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

Expand All @@ -31,6 +32,7 @@
/**
* All requests are serviced by this Servlet, though it usually delegates to other Servlets.
*/
@MultipartConfig
public class WorkbenchGateway extends AbstractServlet {

private static final String DEFAULT_SERVER = "default-server";
Expand Down
54 changes: 31 additions & 23 deletions tools/workbench/src/main/java/org/eclipse/rdf4j/workbench/util/WorkbenchRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,15 @@
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;

import javax.servlet.ServletException;
import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletRequestWrapper;
import javax.servlet.http.Part;

import org.apache.commons.fileupload.FileItemIterator;
import org.apache.commons.fileupload.FileItemStream;
import org.apache.commons.fileupload.FileUploadException;
import org.apache.commons.fileupload.servlet.ServletFileUpload;
import org.eclipse.rdf4j.model.IRI;
import org.eclipse.rdf4j.model.Resource;
import org.eclipse.rdf4j.model.Value;
Expand All @@ -40,7 +39,7 @@
import org.eclipse.rdf4j.workbench.exceptions.BadRequestException;

/**
* Request wrapper used by {@link org.eclipse.rdf4j.workbench.base TransformationServlet}.
* Request wrapper used by {@link org.eclipse.rdf4j.workbench.base.TransformationServlet}.
*/
public class WorkbenchRequest extends HttpServletRequestWrapper {

Expand All @@ -62,22 +61,36 @@ public class WorkbenchRequest extends HttpServletRequestWrapper {
* @param defaults application default parameter values
* @throws RepositoryException if there is an issue retrieving the parameter map
* @throws IOException if there is an issue retrieving the parameter map
* @throws FileUploadException if there is an issue retrieving the parameter map
* @throws ServletException if there is an issue retrieving the parameter map
*/
public WorkbenchRequest(Repository repository, HttpServletRequest request, Map<String, String> defaults)
throws RepositoryException, IOException, FileUploadException {
throws RepositoryException, IOException, ServletException {
super(request);
this.defaults = defaults;
this.decoder = new ValueDecoder(repository,
(repository == null) ? SimpleValueFactory.getInstance() : repository.getValueFactory());
String url = request.getRequestURL().toString();
if (ServletFileUpload.isMultipartContent(this)) {
if (isMultipartContent(this)) {
parameters = getMultipartParameterMap();
} else if (request.getQueryString() == null && url.contains(";")) {
parameters = getUrlParameterMap(url);
}
}

private static boolean isMultipartContent(HttpServletRequest request) {
if (!"POST".equalsIgnoreCase(request.getMethod())) {
return false;
}
String contentType = request.getContentType();
if (contentType == null) {
return false;
}
if (contentType.toLowerCase(Locale.ENGLISH).startsWith("multipart/")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the multipart/ must always be lowercase when coming from a header so my gut feeling is that enforcing a lowercase is not needed and may even be a point of failure in the future. e.g. badly configured method filtering on a proxy assuming the lowercase rule being by passed by accepting uppercase headers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just in-lined the code from commons-fileupload when removing the dependency, to ensure we don't break anything. Is that a good enough argument? IMO better safe than sorry, even if I tend to agree with you. ;-)

return true;
}
return false;
}

/**
* Get the content of any uploaded file that is part of this request.
*
Expand Down Expand Up @@ -265,26 +278,21 @@ public Value getValue(String name) throws BadRequestException, RepositoryExcepti
return decoder.decodeValue(getParameter(name));
}

private String firstLine(FileItemStream item) throws IOException {
try (BufferedReader reader = new BufferedReader(new InputStreamReader(item.openStream()))) {
return reader.readLine();
}
}

private Map<String, String> getMultipartParameterMap()
throws RepositoryException, IOException, FileUploadException {
throws RepositoryException, IOException, ServletException {
Map<String, String> parameters = new HashMap<>();
ServletFileUpload upload = new ServletFileUpload();
FileItemIterator iter = upload.getItemIterator(this);
while (iter.hasNext()) {
FileItemStream item = iter.next();
String name = item.getFieldName();
for (Part part : getParts()) {
String name = part.getName();
if ("content".equals(name)) {
content = item.openStream();
contentFileName = item.getName();
content = part.getInputStream();
contentFileName = part.getSubmittedFileName();
break;
} else {
parameters.put(name, firstLine(item));
String firstLine;
try (BufferedReader reader = new BufferedReader(new InputStreamReader(part.getInputStream()))) {
firstLine = reader.readLine();
}
parameters.put(name, firstLine);
}
}
return parameters;
Expand Down