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.

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

No originalFilename in MockMultipartFile now leads to add it just as part #26261

Closed
markusheiden opened this issue Dec 12, 2020 · 8 comments · Fixed by devhh/spring-framework#2
Assignees
Labels
in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) type: regression A bug that is also a regression
Milestone

Comments

@markusheiden
Copy link
Contributor

When using MockMultipartFile with an empty originalFilename (""), it now gets added to multi part requests just as a part, not as a file. When migrating from Spring Boot 2.3 to 2.4.1, this made my MockMvc test fail for a controller using a @RequestPart MultipartFile parameter causing a longer debugging session :-) It would be nice, if there had been a check (exception), that empty originalFilenames are no longer supported for MockMultipartFiles.

If the nullability of the originalFilename in MultipartFile is intended, org.springframework.web.multipart.support.StandardMultipartHttpServletRequest#parseRequest seems to be broken as it recognizes parts without filename not as files, so that they don't match MultipartFile parameters.

@markusheiden markusheiden changed the title No filename in MockMultipartFile now leads to add it just as part No originalFilename in MockMultipartFile now leads to add it just as part Dec 12, 2020
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 12, 2020
@jhoeller jhoeller self-assigned this Dec 13, 2020
@jhoeller jhoeller added in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Dec 13, 2020
@jhoeller jhoeller added this to the 5.3.3 milestone Dec 13, 2020
@jhoeller
Copy link
Contributor

jhoeller commented Dec 13, 2020

This seems to be a consequence of #25602. I'll revisit the assertions in MockMultipartFile, we should indeed reject empty filenames there then. It's just not entirely clear to me yet where specifically we're evaluating MockMultipartFile-specified empty original filenames to plain parts...

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Dec 14, 2020

This is likely impacted by the more recent change in #26166. There is also a separate new report https://github.com/spring-projects/spring-boot/issues/24483.

@jhoeller
Copy link
Contributor

Good catch, @rstoyanchev - so that effect seems to come from the intermediate MockPart handling since there is filename differentiation being made for Part (but not for MultipartFile which is always clearly a file, never a parameter). I guess this is to be treated as a regression then...

@jhoeller jhoeller added type: regression A bug that is also a regression and removed type: enhancement A general enhancement labels Dec 14, 2020
@rstoyanchev rstoyanchev assigned rstoyanchev and unassigned jhoeller Dec 14, 2020
@jhoeller jhoeller added in: web Issues in web modules (web, webmvc, webflux, websocket) and removed in: web Issues in web modules (web, webmvc, webflux, websocket) labels Dec 14, 2020
timovandeput pushed a commit to philips-software/bom-bar that referenced this issue Dec 17, 2020
Caused by bug in Spring MockMvc multipart file upload:
spring-projects/spring-framework#26261
jhoeller added a commit that referenced this issue Dec 17, 2020
@yankee42
Copy link
Contributor

After a long debug session about the same problem after upgrading from Spring 5.2.12 to 5.3.2 and following git blame, I ended up here :-).

When using MockMultipartFile with an empty originalFilename (""), it now gets added to multi part requests just as a part, not as a file.

Just to clarify... (and document this for other people googling this issue): In version 5.3.2 this was always converted to a part, not just if the originalFilename was empty:

In my case I first got a org.springframework.web.multipart.support.MissingServletRequestPartException: Required request part 'file' is not present.

After giving the file an "originalFilename" I got an UnsupportedOperationException because I was calling MultipartFile.write which delegates to StandardMultipartFile.transferTo which in turn delegates to MockPart.write which always throws. Reading the code associated with this ticket, I think that should be fixed now, but I did not test, yet.

@rstoyanchev
Copy link
Contributor

Thanks for the comment @yankee42 and sorry for the time spent. If you are able to verify the fix with 5.3.3-SNAPSHOT via https://repo.spring.io/snapshot that would be great.

@yankee42
Copy link
Contributor

yankee42 commented Jan 7, 2021

If you are able to verify the fix with 5.3.3-SNAPSHOT via https://repo.spring.io/snapshot that would be great.

I did so and confirm that this is fixed in 5.3.3-SNAPSHOT.

@liorderei
Copy link

is there a workaround for this issue on 2.4.1? I also get it for MultipartFile and not just for MockMultipartFile

@narugoyal
Copy link

is there a workaround for this issue on 2.4.1? I also get it for MultipartFile and not just for MockMultipartFile

For me, using constructor with passing some originalFilename works.

public MockMultipartFile( String name, @Nullable String originalFilename, @Nullable String contentType, @Nullable byte[] content)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) type: regression A bug that is also a regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants