Skip to content

WW-5546 Fixes NPE when uploaded file is empty #1262

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

Open
wants to merge 1 commit into
base: release/struts-6-7-x
Choose a base branch
from
+127 −1
Open

WW-5546 Fixes NPE when uploaded file is empty #1262

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
2 changes: 1 addition & 1 deletion core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ protected boolean acceptFile(Object action, UploadedFile file, String originalFi
}

// If it's null the upload failed
if (file == null) {
if (file == null || file.getContent() == null) {
String errMsg = getTextMessage(action, STRUTS_MESSAGES_ERROR_UPLOADING_KEY, new String[]{inputName});
if (validation != null) {
validation.addFieldError(inputName, errMsg);
Expand Down
63 changes: 63 additions & 0 deletions core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,53 @@ public String getInputName() {
}
};

public static final UploadedFile NULL_CONTENT = new UploadedFile() {
@Override
public Long length() {
return 0L;
}

@Override
public String getName() {
return "";
}

@Override
public boolean isFile() {
return false;
}

@Override
public boolean delete() {
return false;
}

@Override
public String getAbsolutePath() {
return null;
}

@Override
public File getContent() {
return null;
}

@Override
public String getOriginalName() {
return null;
}

@Override
public String getContentType() {
return null;
}

@Override
public String getInputName() {
return null;
}
};

private ActionFileUploadInterceptor interceptor;
private File tempDir;

Expand Down Expand Up @@ -205,6 +252,22 @@ public void testAcceptFileWithNoFile() {
assertTrue(msg.indexOf("inputName") > 0);
}

public void testAcceptFileWithNoContent() {
interceptor.setAllowedTypes("text/plain");

ValidationAwareSupport validation = new ValidationAwareSupport();
boolean notOk = interceptor.acceptFile(validation, NULL_CONTENT, "filename.html", "text/plain", "inputName");

assertFalse(notOk);
assertFalse(validation.getFieldErrors().isEmpty());
assertTrue(validation.hasErrors());
List<String> errors = validation.getFieldErrors().get("inputName");
Copy link
Contributor

Choose a reason for hiding this comment

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

just a small suggestion to use AssertJ:

    assertThat(validation.getFieldErrors().get("inputName"))
        .hasSize(1).first(STRING)
        .startsWith("Error uploading:")
        .contains("inputName");

Copy link
Member Author

Choose a reason for hiding this comment

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

I want to make this change as small as possible, I will do a large refactoring once cherry-picking changes into Struts 7

assertEquals(1, errors.size());
String msg = errors.get(0);
assertTrue(msg.startsWith("Error uploading:"));
assertTrue(msg.indexOf("inputName") > 0);
}

public void testAcceptFileWithMaxSize() throws Exception {
interceptor.setMaximumSize(10L);

Expand Down
63 changes: 63 additions & 0 deletions core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,53 @@ public String getInputName() {
}
};

public static final UploadedFile NULL_CONTENT = new UploadedFile() {
@Override
public Long length() {
return 0L;
}

@Override
public String getName() {
return "";
}

@Override
public boolean isFile() {
return false;
}

@Override
public boolean delete() {
return false;
}

@Override
public String getAbsolutePath() {
return null;
}

@Override
public File getContent() {
return null;
}

@Override
public String getOriginalName() {
return null;
}

@Override
public String getContentType() {
return null;
}

@Override
public String getInputName() {
return null;
}
};

private FileUploadInterceptor interceptor;
private File tempDir;

Expand Down Expand Up @@ -202,6 +249,22 @@ public void testAcceptFileWithNoFile() {
assertTrue(msg.indexOf("inputName") > 0);
}

public void testAcceptFileWithNoContent() {
interceptor.setAllowedTypes("text/plain");

ValidationAwareSupport validation = new ValidationAwareSupport();
boolean notOk = interceptor.acceptFile(validation, NULL_CONTENT, "filename.html", "text/plain", "inputName");

assertFalse(notOk);
assertFalse(validation.getFieldErrors().isEmpty());
assertTrue(validation.hasErrors());
List<String> errors = validation.getFieldErrors().get("inputName");
assertEquals(1, errors.size());
String msg = errors.get(0);
assertTrue(msg.startsWith("Error uploading:"));
assertTrue(msg.indexOf("inputName") > 0);
}

public void testAcceptFileWithMaxSize() throws Exception {
interceptor.setMaximumSize(10L);

Expand Down