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

Upload is broken for V12+Spring, and V10+SpringMVC #5924

Closed
pleku opened this issue Nov 14, 2018 · 41 comments
Closed

Upload is broken for V12+Spring, and V10+SpringMVC #5924

pleku opened this issue Nov 14, 2018 · 41 comments
Assignees
Milestone

Comments

@pleku
Copy link
Contributor

pleku commented Nov 14, 2018

@denis-anisimov commented on Tue Sep 25 2018

The original issue was : #59.

Upload still doesn't work: at least its succeeded listener is not called when @EnableWebMvc is used in config.
It's called correctly and everything works fine if there is no @EnableWebMvc annotation.

It might be that this is similar to #24.


@denis-anisimov commented on Tue Sep 25 2018

Apparently the problem is the multipart resolver.
If multipart resolver is configured via

@Bean(name = "multipartResolver")
    public CommonsMultipartResolver multipartResolver() {
        return new CommonsMultipartResolver();
    }

then upload doesn't work regardless of @EnableWebMvc annotation.


@denis-anisimov commented on Tue Sep 25 2018

If there is no @EnableWebMvc annotation then Spring uses StandardServletMutipartResolver somehow configured by default.

It's tricky to configure this resolver by yourself.

It looks like the behavior of CommonsMultipartResolver is inconsistent with commons-file-upload.


@heruan commented on Tue Sep 25 2018

What if vaadin-spring module would provide the resolver? It could be annotated with @ConditionalOnMissingBean so it is used only if the user does not provide their own.

The resolver provided by Vaadin would then have reasonable defaults, possibly configurable via application.properties.

I guess this is not only related to Upload, but also to other means of using StreamReceiver in Flow.


@denis-anisimov commented on Tue Sep 25 2018

I believe you are talking about StandardServletMutipartResolver.

It's not that easy. This resolver requires additional MultipartConfigElement which needs to be somehow provided.
One of the way of doing this is set it for ServletRegistration.Dynamic whose instance is available in the vaadin spring configuration only.

Javadocs says that there is another way of doing this : AbstractAnnotationConfigDispatcherServletInitializer subclass which set the element.

In the simplest way the developer won't be able to configure the resolver (such as maximum uploaded file size and so on).

I've been thinking about this. If I find a good way to make it configurable then I will do it.

But it's still worth to understand why "apache" CommonsMultipartResolver doesn't want to work with apache commons-file-upload. This is nonsense.


@denis-anisimov commented on Wed Sep 26 2018

I think this is a bug in Spring.
I've found similar issue spring-projects/spring-boot#7735.
But :

  • it's fixed
  • I can't get it working with suggested workarounds which disables Servlet 3.x multipart support.

About using StandardServletMutipartResolver by default if there is no custom multipart resolver defined.

I'm not able to get it working at all if @EnableWebMvc annotation is used. It requires additional MultipartConfigElement set but it fails every time when I set it for any servlet registration.
So it's still a big issue.


@denis-anisimov commented on Wed Sep 26 2018

So interesting thing....

The presence of CommonsMultipartResolver bean breaks upload totally.

StandardServletMutipartResolver may be used explicitly declared as a multipart resolver bean (with additional MultipartConfigElement config ). It works without @EnableWebMvc .
But it again throws an exception (another one) if there is @EnableWebMvc.
WTH.


@heruan commented on Wed Sep 26 2018

Thank you Denis for being into this! I'm following your updates and doing tests too on my side, and yeah this Spring behaviour is quite irritating…

For example I do not use @EnableWebMvc but still upload doesn't work, so I guess I have some dependency which enables that under the hood—one more reason to tackle this down once for all!

Spring has also a bunch of configuration properties to set in application.yml, i.e.

spring:
  servlet:
    multipart:
      enabled: true
      file-size-threshold:
      max-file-size:
      max-request-size:
      resolve-lazily: 

but what for is it doesn't work? I'm trying now to set them on a non-Vaadin project, to see if outside Vaadin context these properties work.


@heruan commented on Wed Sep 26 2018

Take for example this project: https://github.com/spring-guides/gs-uploading-files

If you run it as is, upload fails because there is spring.servlet.multipart.enabled set to false (spring-guides/gs-uploading-files#47); but setting that to true, upload works well without the need of any explicit MultipartResolver bean.

BUT

if you add vaadin-spring-boot-starter to the project, it stops working! So I'd say the Vaadin Spring module does something in the servlet initialization that breaks uploads.

I have a feeling this might be related to vaadin/spring#331 where @snicoll stated that Vaadin Spring module "overrides a core piece of Spring Boot" in the context of the dispatcher servlet.


@denis-anisimov commented on Wed Sep 26 2018

That might be a consequence of transitive dependencies as well.

But let's check your assumption first. It's not a big deal. I will make a test for non root context Spring app.


@denis-anisimov commented on Wed Sep 26 2018

You are right, @heruan .

Everything works fine if servlet is not mapped to the root meaning there is no hack with the dispatcher servlet (vaadin/spring#331).
So this is just another aspect of vaadin/spring#331.
Which means we need to look at it closer.


@heruan commented on Tue Oct 02 2018

Any update on this? Not sure if this will be resolved with vaadin/spring#331 or before.


@denis-anisimov commented on Tue Oct 02 2018

No any update.

This ticket technically is a duplicate of vaadin/spring#331.
But it's not a duplicate, it's a consequence of the mapping done for the dispatcher servlet which also causes the vaadin/spring#331.
So once we resolve vaadin/spring#331 we will resolve this as well.

But since it's not exactly the same issue as vaadin/spring#331 I have doubts to close it (as a duplicate).

So I keep it open for now.

The progress should be for vaadin/spring#331. But I have no any idea how to fix it.
I don't understand what's going on there. This issue requires deep Spring expertise which I don't have.
So please just track progress in vaadin/spring#331 but it's not progressing, it's not prioritizes and I don't have any thoughts how to solve it.


@andrea-tomassi commented on Tue Oct 02 2018

Not sure you still have the problem.
My experience (vaadin Flow 10) is that the upload is not working if you use:


@Bean(name = "multipartResolver")
    public CommonsMultipartResolver multipartResolver() {
        return new CommonsMultipartResolver();
    }

If you delete the multipartResolver Bean from Spring @configuration the upload works well out of the box! Just add the component and add the listener. Stop.
It seems that now the multipartResolver Bean declaration is part of the base Vaadin project, and adding your own declaration breaks the upload.


@denis-anisimov commented on Wed Oct 03 2018

The reason why it doesn't work is described here: vaadin/vaadin-upload-flow#84 (comment)

It works fine with or without explicitly provided mutlipart resolver in case if you servlet is mapped to non root context.


@pleku commented on Wed Oct 03 2018

So please just track progress in vaadin/spring#331 but it's not progressing, it's not prioritizes and I don't have any thoughts how to solve it.

@heruan the Spring issue will move forward in the next four weeks. I'm sorry but that is all I can say at the moment, we have other priorities at the moment.


@heruan commented on Wed Oct 03 2018

Thank you @pleku for the feedback. Does this mean it will be fixed in 12?


@pleku commented on Wed Oct 03 2018

Yes, that is the target at the moment.


@denis-anisimov commented on Fri Oct 05 2018

The test for usecase is provided and disabled at the moment.
Here is the ticket for enabling it back once the ticket is fixed:
vaadin/spring#359


@heruan commented on Tue Oct 30 2018

I've got upload working updating to Spring Boot 2.1.0.M4 and Vaadin 12.0.0.alpha2!

(Nevermind, I didn't have @EnableWebMvc, which still breaks uploads.)


@denis-anisimov commented on Mon Oct 08 2018

(Nevermind, I didn't have @EnableWebMvc, which still breaks uploads.)

:) Exactly.


@priyamalM commented on Mon Oct 08 2018

any updates on this issue ??


@heruan commented on Tue Oct 09 2018

Worth noting that on WildFly 14 it fails also without @EnableWebMvc:

java.lang.IllegalStateException: UT010057: multipart config was not present on Servlet
	at io.undertow.servlet.spec.HttpServletRequestImpl.verifyMultipartServlet(HttpServletRequestImpl.java:520)
	at io.undertow.servlet.spec.HttpServletRequestImpl.getParts(HttpServletRequestImpl.java:509)
	at javax.servlet.http.HttpServletRequestWrapper.getParts(HttpServletRequestWrapper.java:390)
	at org.springframework.web.multipart.support.StandardMultipartHttpServletRequest.parseRequest(StandardMultipartHttpServletRequest.java:94)

@denis-anisimov commented on Mon Nov 05 2018

The latest update:

  • we have removed a workaround in Flow core related to the dispatcher servlet.
  • now dispatcher servlet is not overridden
  • but the Upload issue is still there

As a result: it looks like the Upload issue is not related to the dispatcher servlet itself.
The issue is caused by the forwarding workaround which we have to apply in case of root mapping.
I don't know how it affects the Upload exactly but apparently it does.

I was thinking that it may be fixed by one more change which exists in FW8 and is related to the forwarding workaround :
https://github.com/vaadin/spring/blob/3.0/vaadin-spring/src/main/java/com/vaadin/spring/server/SpringVaadinServletService.java#L69

(Note that everything else perfectly works without this change so it's not clear whether it's needed or not).

The �serviceUrl value in this code is /vaadinServlet the original mapping which is used in root mapping case (before forwarding is applied).
I've changed the getServiceUrl method return value to /vaadinServlet and checked whether
Upload works with this change (the method is used inside the setupMetaAndTitle method to set base href value).

It didn't help.
May be this info is irrelevant but I wanted to save it here just in case.


@heruan commented on Wed Nov 14 2018

With the latest Spring add-on, Upload does not work even without @EnableWebMvc 😕

So now with 12.0.0.beta1 uploads simply don't work with Spring, which is not just a Upload issue but any StreamReceiver (which is Flow core).

Feels like this should be moved (or open a new one) in the Spring add-on repo, as the title "[Spring] Upload listeners are not called if multipart resolver is provided and @EnableWebMvc is used in config" doesn't reflect much the current status:

  • it's not about upload listeners, but about the request handler;
  • the multipart resolver isn't relevent, as its presence doesn't make a difference;
  • nor is the annotation @EnableWebMvc, as uploads do not work regardless of it.
@pleku pleku changed the title [Spring] Upload listeners are not called if multipart resolver is provided and @EnableWebMvc is used in config Upload is broken for V12+Spring, and V10+SpringMVC Nov 14, 2018
@psporysz
Copy link

I've been trying get the Upload working for both Vaadin 11 and 12. I started with "Project Base with Spring" and followed the Upload documentation. Unfortunately it does not make any difference whether the multipartResolver bean is defined or not.

As work-around i disabled the multipart spring servlet:

spring:
  servlet:
    multipart:
      enabled: false

and using Vaadin 12.0.0.beta2 with Spring 2.1.0.RELEASE. No multipart bean is overridden as stated in documentation and Upload works fine.

@heruan
Copy link
Member

heruan commented Nov 21, 2018

@psporysz what do you mean with "work-around"? What does change disabling multipart?

@psporysz
Copy link

It is working then :) The multipart is handled correctly after disabled in spring. I tested it for both the MultiFileMemoryBuffer and MemoryBuffer and it behaves exactly the same (e.g. without multipart disabled in Spring the succeeded listener is not fired).

The problem is if someone will try to start Spring project out of Vaadin project base and will follow the upload documention he will suffer the issue mentioned here and hence the work-around.

@heruan
Copy link
Member

heruan commented Nov 22, 2018

Thank you for the feedback! I'm asking because I'm not able to make it work, even disabling Spring multipart. Do you provide a MultipartResolver bean then? Can you share a simple working project here on GitHub?

@heruan
Copy link
Member

heruan commented Nov 22, 2018

Nevermind, I was able to replicate your workaround:

  • spring.servlet.multipart.enabled = false
  • do not provide a MultipartResolver

and uploads work! I'll try to test it in all the scenarios that were failing and report back here.

@psporysz
Copy link

Yes, in the mean time i created the https://github.com/psporysz/vaadin-spring-demo :)

@denis-anisimov
Copy link
Contributor

Cool......
This is important information.

Does upload work for you with this config if you use it for upload multiple files ?

@psporysz
Copy link

@denis-anisimov yes, it works for MultiFileMemoryBuffer just fine -- StreamReceiverHandler is parsing the request correctly.

@denis-anisimov
Copy link
Contributor

OK, cool.
Thank you very much for the information.

@heruan
Copy link
Member

heruan commented Nov 22, 2018

@psporysz Are you also able to read from the files? I'm getting java.io.IOException: Stream Closed when reading the stream returned by receiver.getInputStream(fileName).

@heruan
Copy link
Member

heruan commented Nov 22, 2018

It works with MultiFileMemoryBuffer, so the exception could be a bug (or my misuse) in MultiFileBuffer which is not relevant here.

@psporysz
Copy link

@heruan yes, i do use this component in my project and can read multiple files via receiver.getInputStream(filename), i've updated the https://github.com/psporysz/vaadin-spring-demo, there is /multipart-upload example where it does work.

@pleku
Copy link
Contributor Author

pleku commented Nov 22, 2018

Picking this for next Sprint for further investigation, and need to revert the workarounds added in vaadin/skeleton-starter-flow-spring#100

@Andelous
Copy link

Nevermind, I was able to replicate your workaround:

  • spring.servlet.multipart.enabled = false
  • do not provide a MultipartResolver

and uploads work! I'll try to test it in all the scenarios that were failing and report back here.

Same problem here. Migrated from V10 to V12 and got stuck with this issue, but fortunately the workaround still works.

@eiswind
Copy link

eiswind commented Jan 2, 2019

Unfortunatley this "workaround" disables Mutlipartuploads to Spring MVC Controllers! So in my case it's just the problem elsewhere.

@lbruun
Copy link
Contributor

lbruun commented Jan 25, 2019

Just a long shot here:

Spring Boot autoconfigures a HiddenHttpMethodFilter. This has the side-effect that it consumes the body of a POST request so that when Vaadin gets there the body has already been consumed. Sounds bad to me.

Since Spring Boot v2.1.2 this can now be turned off by setting:

spring.mvc.hiddenmethod.filter.enabled=false

I believe the idea of HiddenHttpMethodFilter is irrelevant in a Vaadin application and therefore it can safely be turned off. Give it a try. (no promises, but worth a shot, I think)

@luca-lezzerini
Copy link

I've tried with Vaadin 13.0.0 and Spring Boot 2.1.3.RELEASE and upload listeners doesn't fire when I activate upload button.
I've used the suggested workarounds and tried with previous versions of Spring Boot and Vaadin but it is not working.
Anny Idea?

@luca-lezzerini
Copy link

I've also found a similar problem on Stack Overflow: same behaviour I met.

@luca-lezzerini
Copy link

Another update: the problem is solved returning to Vaadin 10.0.6 and Spring Boot 2.0.3 as stated in this thread.
I tried doing this and it worked!
Then I changed to Vaadin 10.0.7, keeping Spring Boot 2.0.3, and it worked again.
Changing Spring Boot to 2.0.4, TomCat didn't start.
Changing to Vaadin 13.0.0 and Spring Boot 2.0.3 it worked again.
Vaadin 13.0.0 and SpringBoot 2.0.4 TomCat crashed again.
Vaadin 13.0.0 and SpringBoot 2.0.5 TomCat crashed again.
Vaadin 13.0.0 and SpringBoot 2.1.0 got exception.

@festaumberto
Copy link

Another update: the problem is solved returning to Vaadin 10.0.6 and Spring Boot 2.0.3 as stated in this thread.
I tried doing this and it worked!
Then I changed to Vaadin 10.0.7, keeping Spring Boot 2.0.3, and it worked again.
Changing Spring Boot to 2.0.4, TomCat didn't start.
Changing to Vaadin 13.0.0 and Spring Boot 2.0.3 it worked again.
Vaadin 13.0.0 and SpringBoot 2.0.4 TomCat crashed again.
Vaadin 13.0.0 and SpringBoot 2.0.5 TomCat crashed again.
Vaadin 13.0.0 and SpringBoot 2.1.0 got exception.

I'm using Vaadin 13.0.0 and SpringBoot 2.0.3 , the upload event is not triggered. I was on SpringBoot 2.1.3 , same problem.
I also tried to disable the HiddenHttpMethodFIlter, same result on 2.1.3 and 2.0.3.
I need the multipart servlet enabled because I need to expose REST APIs to upload files, so I really need a fix for this problem.

@jetdario
Copy link

jetdario commented Apr 5, 2019

Has anyone have a work around with this bug? I am already using some Vaadin 13 components, so it's quite difficult to downgrade with some lower version, especially with 8.

@festaumberto
Copy link

Has anyone have a work around with this bug?

afaik, the only workaround available is to disable the multipart servlet ...
in application.properties:
spring.servlet.multipart.enabled=false

@eiswind
Copy link

eiswind commented Apr 5, 2019

I could make everything work be declaring a CommonsMultipartResolver Bean. This is compatible with Vaadin and makes the default resolver back off.

@festaumberto
Copy link

CommonsMultipartResolver

so the combination is to disable the multipart serlet, then declaring a CommonsMultipartResolver bean?

@jetdario
Copy link

jetdario commented Apr 5, 2019

Has anyone have a work around with this bug?

afaik, the only workaround available is to disable the multipart servlet ...
in application.properties:
spring.servlet.multipart.enabled=false

I already have done that, but I can only get one response from upload events, after that, even Sout.Println wont show any output, unless I have to restart the server.

@eiswind
Copy link

eiswind commented Apr 5, 2019

Just defining the CommonsMultiPartResolver should be enough, as the AutoConfiguration recognizes that.
That way you I could do uploads with Vaadin as well as MVC.

@festaumberto
Copy link

Has anyone have a work around with this bug?

afaik, the only workaround available is to disable the multipart servlet ...
in application.properties:
spring.servlet.multipart.enabled=false

I already have done that, but I can only get one response from upload events, after that, even Sout.Println wont show any output, unless I have to restart the server.

That's strange, my events are correctly triggered with this workaround.. I don't know Vaadin so well to help you further

@festaumberto
Copy link

Just defining the CommonsMultiPartResolver should be enough, as the AutoConfiguration recognizes that.
That way you I could do uploads with Vaadin as well as MVC.

Not working for me: Vaadin is not triggering uploads. Can you please provide your CommonsMultiPartResolver bean definition? what do you have in application.properties? I removed all the multipart related properties, same result

@jetdario
Copy link

jetdario commented Apr 5, 2019

Just defining the CommonsMultiPartResolver should be enough, as the AutoConfiguration recognizes that.
That way you I could do uploads with Vaadin as well as MVC.

pardon, but Im kinda lost here.. Im not familiar with CommonsMultiPartResolver. Where and how to implement it.

@eiswind
Copy link

eiswind commented Apr 5, 2019

OK I double checked it. I have
spring.servlet.multipart.enabled=false

Plus (its Kotlin code...)
`
@bean
fun multiPart() {

    val resolver = CommonsMultipartResolver()

    resolver.setMaxUploadSize(TEN_MEGS)

}
`
@bean has uppercase B, I don't get this editor...

@festaumberto
Copy link

OK I double checked it. I have
spring.servlet.multipart.enabled=false

Plus (its Kotlin code...)
`
@bean
fun multiPart() {

    val resolver = CommonsMultipartResolver()

    resolver.setMaxUploadSize(TEN_MEGS)

}
`
that in java wuold be something like

@bean
public CommonsMultipartResolver getResolver() throws IOException {
CommonsMultipartResolver resolver = new CommonsMultipartResolver();
resolver.setMaxUploadSize(52428807);
return resolver;
}

not working for me, uploads are OK with postman but Vaadin is not working

@eiswind
Copy link

eiswind commented Apr 5, 2019

Sorry I have no idea. The MultiPartResover is for MVC uploads only. Working like a charm at my side.

@festaumberto
Copy link

Sorry I have no idea. The MultiPartResover is for MVC uploads only. Working like a charm at my side.

thank you for your time

@jetdario
Copy link

jetdario commented Apr 5, 2019

upload events is not responding when using System.out.print() but works using Notification component.. weird!

@festaumberto
Copy link

upload events is not responding when using System.out.print() but works using Notification component.. weird!

fascinating!

@elhoce
Copy link

elhoce commented Apr 12, 2019

same result with vaadin 13.0.3 & spring boot = 2.0.8.RELEASE
succeed event is not fired.

edit : Not works with 2.0.3

@elhoce
Copy link

elhoce commented Apr 12, 2019

for those who don't need multipart support
spring.servlet.multipart.enabled=false
works for me !

@peholmst
Copy link
Member

Unless I have missed something, I believe the underlying issue is in Flow, not in Vaadin Spring. I have created a PR in the flow repository.

@pleku pleku transferred this issue from vaadin/spring Jun 17, 2019
@pleku pleku added this to the 1.4.6 milestone Jun 17, 2019
@pleku pleku mentioned this issue Jun 17, 2019
@DSanborn
Copy link

DSanborn commented May 6, 2020

I've upgraded my project from 14 t o 15.0.5 and am still experiencing this issue. Adding spring.servlet.multipart.enabled=false to my application.properties file did not help.

@DSanborn
Copy link

DSanborn commented May 6, 2020

Within upload constructor "target" attribute is being set but attribute is never present, therefor error is thrown 100% of the time.

public Upload() {

 getElement().setAttribute("target", new StreamReceiver(
                getElement().getNode(), "upload", getStreamVariable()));      
public Element setAttribute(String attribute,
            AbstractStreamResource resource) {
 if (!customAttribute.isPresent()) {
            getStateProvider().setAttribute(getNode(), attribute, resource);
        } else {
            throw new IllegalArgumentException("Can't set " + attribute
                    + " to StreamResource value. This attribute has special semantic");
        }

@DSanborn
Copy link

DSanborn commented May 7, 2020

I've noticed that this error occurs when my class is decorated with the @component annotation, but not the @route annotation. I however don't desire to use the @route annotation since I am simply replacing the components in a div when the next/back button is selected.

I'm using @SpringBootApplication and removed @componentscan and used scanBasePackages instead. At the end of the day I used @route and RouterLink to then swap my components.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests