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

HtmlUnitRequestBuilder ignores file uploaded via HtmlFileInput.setData() #27199

Closed
svschouw-bb opened this issue Jul 22, 2021 · 7 comments
Closed

HtmlUnitRequestBuilder ignores file uploaded via HtmlFileInput.setData() #27199

svschouw-bb opened this issue Jul 22, 2021 · 7 comments
Assignees
@sbrannen
Labels
in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug
Milestone
5.3.10

Comments

@svschouw-bb
Copy link

When testing an application using HtmlUnit, its often useful to use HtmlFileInput.setData(...) instead of HtmlFileInput.setFiles(...) in order to not have to generate temporary files. However, when using MockMvcWebClientBuilder, this doesn't work. The problem is that HtmlUnitRequestBuilder ignores the data part.

#24926 added support for file uploads in HtmlUnitRequestBuilder, but missed this use case.

Test case:

filesubmit.html:

<!DOCTYPE html>
<html>
<head>
<title>File submission test</title>
</head>
<body>
<form method="POST" action="/filesubmit" enctype="multipart/form-data">
	File: <input name="file" type="file"/>
	<button type="submit">Submit</button>
</form>
</body>
</html>

FileSubmitController.java:

@Controller
public class FileSubmitController {
	@PostMapping("/filesubmit")
	public @ResponseBody String handleFileSubmit(@RequestParam(name = "file", required = false) MultipartFile file) {
		return file == null ? "unset" : file.getOriginalFilename();
	}
}

FileSubmitTest.java:

@WebMvcTest(FileSubmitController.class)
public class FileSubmitTest {
	@Autowired
	WebClient webClient;

	@Test
	void testFileSubmitUsingFile() throws FailingHttpStatusCodeException, MalformedURLException, IOException {
		HtmlPage page = webClient.getPage("/filesubmit.html");
		HtmlFileInput file = page.querySelector("input[type=file]");
		file.setContentType("application/json");
		file.setFiles(new File("src/test/resources/test.json"));
		HtmlButton submit = page.querySelector("button");
		Page resultPage = submit.click();
		// Succeeds
		assertEquals("test.json", resultPage.getWebResponse().getContentAsString());
	}

	@Test
	void testFileSubmitUsingData() throws FailingHttpStatusCodeException, MalformedURLException, IOException {
		HtmlPage page = webClient.getPage("/filesubmit.html");
		HtmlFileInput file = page.querySelector("input[type=file]");
		file.setValueAttribute("test.json");
		file.setContentType("application/json");
		file.setData("{}".getBytes());
		HtmlButton submit = page.querySelector("button");
		Page resultPage = submit.click();
		// Fails with expected: <test.json> but was: <unset>
		assertEquals("test.json", resultPage.getWebResponse().getContentAsString());
	}
}

When starting the application normally and changing WebClient from a mocked version to a normal version using http://localhost:8080/ both tests succeed.

I'm guessing in HtmlUnitRequestBuilder.params(...), the else part should be something like this:

				else { // use data
					part = new MockPart(pair.getName(), pair.getValue(), pair.getData());
					if (pair.getMimeType() == null)
						part.getHeaders().setContentType(MediaType.APPLICATION_OCTET_STREAM);
					else
						part.getHeaders().setContentType(MediaType.valueOf(pair.getMimeType()));
				}

I'm not sure if there is a specific edge case to use the old "mimic empty file upload" else case.

Tested using Spring Boot 2.5.2 using Spring WebMVC 5.3.8.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jul 22, 2021
@sbrannen sbrannen added in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Aug 3, 2021
@sbrannen sbrannen added this to the 5.3.10 milestone Aug 3, 2021
@sbrannen sbrannen self-assigned this Aug 3, 2021
@sbrannen
Copy link
Member

sbrannen commented Aug 3, 2021

I'm not sure if there is a specific edge case to use the old "mimic empty file upload" else case.

I think that use case will still exist, resulting in 3 use cases.

  1. file is set
  2. data is set
  3. neither file nor data is set (empty upload)

@sbrannen sbrannen changed the title HtmlUnitRequestBuilder ignores HtmlFileInput.setData(...)/KeyDataPair.getData() HtmlUnitRequestBuilder ignores file uploaded via HtmlFileInput.setData() Aug 22, 2021
@sbrannen
Copy link
Member

Related issues:

@sbrannen
Copy link
Member

I'm not sure if there is a specific edge case to use the old "mimic empty file upload" else case.

Well, after diving into the implementation, it turns out you were right about that. 👍

I was able to support both of those use cases in a single else block. See 8a7c4fc for details.

I would be grateful if you could try out the fix in one of the upcoming 5.3.10 snapshots and let us know if everything works as expected.

Cheers!

@svschouw-bb
Copy link
Author

I would be grateful if you could try out the fix in one of the upcoming 5.3.10 snapshots and let us know if everything works as expected.

I'd love to, but I'm not sure how. Where and when will these snapshots be available?

@sbrannen
Copy link
Member

I'd love to, but I'm not sure how. Where and when will these snapshots be available?

The 5.3.10 snapshots for spring-test are available here.

Information on how to consume snapshots can be found here.

@svschouw-bb
Copy link
Author

Yes, seems to work! Great!

@sbrannen
Copy link
Member

Yes, seems to work! Great!

Awesome!

Thanks for letting us know.

@sync-by-unito sync-by-unito bot mentioned this issue Sep 15, 2021
Closed
lxbzmy pushed a commit to lxbzmy/spring-framework that referenced this issue Mar 26, 2022
@sbrannen @lxbzmy
Support HtmlFileInput.setData() with HtmlUnit and MockMvc
7839cc9 
Prior to this commit, if the user tested file upload support with
HtmlUnit and MockMvc by invoking HtmlFileInput.setData() instead of
HtmlFileInput.setFiles(), the in-memory file data was simply ignored.

This commit addresses this issue by creating a MockPart from the
in-memory data in HtmlUnitRequestBuilder.

Closes spring-projectsgh-27199
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sbrannen sbrannen

Labels
in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug
Projects
None yet
Milestone
5.3.10
Development

No branches or pull requests
3 participants
@sbrannen @svschouw-bb @spring-projects-issues