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

SynchronossPartHttpMessageReader should only create temp directory when needed #27092

Closed
yechao4j opened this issue Jun 22, 2021 · 11 comments
Closed
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches type: enhancement A general enhancement
Milestone

Comments

@yechao4j
Copy link

2021-06-22 11:16:31.394 [reactor-http-epoll-4] ERROR reactor.netty.http.server.HttpServer - [id: 0xc8938d6a, L:/10.202.13.109:8080 - R:/10.202.12.128:35570]
java.io.UncheckedIOException: java.nio.file.FileSystemException: /tmp/synchronoss-file-upload-2054205257255420694: No space left on device
        at org.springframework.http.codec.multipart.SynchronossPartHttpMessageReader.createTempDirectory(SynchronossPartHttpMessageReader.java:202)
        at org.springframework.http.codec.multipart.SynchronossPartHttpMessageReader.<init>(SynchronossPartHttpMessageReader.java:96)
        at org.springframework.http.codec.support.ServerDefaultCodecsImpl.extendTypedReaders(ServerDefaultCodecsImpl.java:71)
        at org.springframework.http.codec.support.BaseDefaultCodecs.getTypedReaders(BaseDefaultCodecs.java:235)
        at org.springframework.http.codec.support.BaseCodecConfigurer.getReaders(BaseCodecConfigurer.java:98)
        at org.springframework.http.codec.support.DefaultServerCodecConfigurer.getReaders(DefaultServerCodecConfigurer.java:27)
        at org.springframework.web.server.adapter.DefaultServerWebExchange.initFormData(DefaultServerWebExchange.java:143)
        at org.springframework.web.server.adapter.DefaultServerWebExchange.<init>(DefaultServerWebExchange.java:131)
        at org.springframework.web.server.adapter.HttpWebHandlerAdapter.createExchange(HttpWebHandlerAdapter.java:252)
        at org.springframework.web.server.adapter.HttpWebHandlerAdapter.handle(HttpWebHandlerAdapter.java:238)
        at org.springframework.boot.web.reactive.context.WebServerManager$DelayedInitializationHttpHandler.handle(WebServerManager.java:97)
        at org.springframework.http.server.reactive.ReactorHttpHandlerAdapter.apply(ReactorHttpHandlerAdapter.java:65)
        at org.springframework.http.server.reactive.ReactorHttpHandlerAdapter.apply(ReactorHttpHandlerAdapter.java:40)
        at reactor.netty.http.server.HttpServerHandle.onStateChange(HttpServerHandle.java:64)
        at reactor.netty.ReactorNetty$CompositeConnectionObserver.onStateChange(ReactorNetty.java:537)
        at reactor.netty.tcp.TcpServerBind$ChildObserver.onStateChange(TcpServerBind.java:278)
        at reactor.netty.http.server.HttpServerOperations.onInboundNext(HttpServerOperations.java:475)
        at reactor.netty.channel.ChannelOperationsHandler.channelRead(ChannelOperationsHandler.java:96)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
        at reactor.netty.http.server.HttpTrafficHandler.channelRead(HttpTrafficHandler.java:191)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
        at io.netty.channel.CombinedChannelDuplexHandler$DelegatingChannelHandlerContext.fireChannelRead(CombinedChannelDuplexHandler.java:436)
        at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:324)
        at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:311)
        at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:432)
        at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:276)
        at io.netty.channel.CombinedChannelDuplexHandler.channelRead(CombinedChannelDuplexHandler.java:251)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
        at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
        at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
        at io.netty.channel.epoll.AbstractEpollStreamChannel$EpollStreamUnsafe.epollInReady(AbstractEpollStreamChannel.java:795)
        at io.netty.channel.epoll.EpollEventLoop.processReady(EpollEventLoop.java:480)
        at io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:378)
        at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:989)
        at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
        at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
        at java.lang.Thread.run(Thread.java:745)
Caused by: java.nio.file.FileSystemException: /tmp/synchronoss-file-upload-2054205257255420694: No space left on device
        at sun.nio.fs.UnixException.translateToIOException(UnixException.java:91)
        at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:102)
        at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:107)
        at sun.nio.fs.UnixFileSystemProvider.createDirectory(UnixFileSystemProvider.java:384)
        at java.nio.file.Files.createDirectory(Files.java:674)
        at java.nio.file.TempFileHelper.create(TempFileHelper.java:136)
        at java.nio.file.TempFileHelper.createTempDirectory(TempFileHelper.java:173)
        at java.nio.file.Files.createTempDirectory(Files.java:991)
        at org.springframework.http.codec.multipart.SynchronossPartHttpMessageReader.createTempDirectory(SynchronossPartHttpMessageReader.java:199)
        ... 44 common frames omitted

I upgrade springboot version to 2.3.12.RELEASE, SpringCloud: Hoxton.SR11, and met error upside. I found SynchronossPartHttpMessageReader default private Path fileStorageDirectory = createTempDirectory(); create temp dir, and never delete it. So my disk space if full.
Why SynchronossPartHttpMessageReader in spring-web-5.2.15.RELEASE doing so? Is it a bug?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 22, 2021
@yechao4j
Copy link
Author

@sdeleuze @rstoyanchev @poutsma @bclozel Please help!

@poutsma
Copy link
Contributor

poutsma commented Jun 22, 2021

There is no reason to ping us directly after filing an issue. It will get the attention it deserves.

As to your question: what you are seeing is the result of #26931, which in term was a fix for this security report. Without this change, Synchronoss creates a fixed temp directory, which has serious security implications. We cannot delete the directory at exit, because that would delete all uploaded files.

In other words: no, this is not a bug.

The disk space of the directory itself is negligible though, it's only if a lot of uploaded files are stored there that the directory will start to take space. And typically, your OS cleans up temporary files after a while.

@poutsma poutsma closed this as completed Jun 22, 2021
@poutsma poutsma added in: web Issues in web modules (web, webmvc, webflux, websocket) status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jun 22, 2021
@RekaDowney
Copy link

@poutsma might be a stupid question.
I found that the temporary directory is not deleted in docker, and the temporary directories are created every time, regardless of whether the Content-Type is multipart/form-data.

I raised an issue on spring-cloud-gateway, I'm not sure where should I raise, docker?

@poutsma
Copy link
Contributor

poutsma commented Jun 23, 2021

@RekaDowney

If you are not using multipart support, then these directories should be empty and only use up a several bytes each.
Can you give me an indication how many synchronoss-file-upload- directories you have in $TMP, and how much disk space they use?

@poutsma poutsma reopened this Jun 23, 2021
@bclozel
Copy link
Member

bclozel commented Jun 23, 2021

Here are some commands that you could run in your host to provide the information asked by @poutsma :

$ df -ih /tmp
$ cd /tmp
$ find synchronoss* -type d | wc -l
$ find synchronoss* -type d -empty | wc -l

Also, could you give us more information about the /tmp file system in your docker image?
Is this mounted as a tmpfs? Is there a cleanup process already in place (like tmpreaper)?

@poutsma poutsma removed the status: invalid An issue that we don't feel is valid label Jun 23, 2021
@poutsma poutsma changed the title Why SynchronossPartHttpMessageReader create /tmp/synchronoss-file-upload-2054205257255420694 SynchronossPartHttpMessageReader should only create temp directory when needed Jun 23, 2021
@poutsma poutsma added type: enhancement A general enhancement for: backport-to-5.2.x Marks an issue as a candidate for backport to 5.2.x labels Jun 23, 2021
@spring-projects-issues spring-projects-issues added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.2.x Marks an issue as a candidate for backport to 5.2.x labels Jun 23, 2021
@poutsma poutsma added this to the 5.3.9 milestone Jun 23, 2021
poutsma added a commit that referenced this issue Jun 23, 2021
The SynchronossPartHttpMessageReader should only create temp directory
when needed, not at startup.

Closes gh-27092
@poutsma
Copy link
Contributor

poutsma commented Jun 23, 2021

I have made some changes, so that the directory is now only created when parsing multipart data. This still means that you can end up with multiple temporary directories, but there should be less of them.

@yechao4j
Copy link
Author

@poutsma Any plan to make this changes in 5.2.x?

@RekaDowney
Copy link

@poutsma

Thanks for your reply.

I'm sorry I can't count the synchronoss-file-upload- directories under /tmp (At least 2 million or more, each empty directory is 4KB), because the pod has been restarted, and I have switched to using a custom SynchronossPartHttpMessageReader to ensure that the directory is created only when the Content-Type is multipart.

The effect is significant, when the QPS grows, the CPU load will not increase much.

I only adjusted a few lines of code. If necessary, I can submit a PR.

@poutsma
Copy link
Contributor

poutsma commented Jun 24, 2021

@poutsma Any plan to make this changes in 5.2.x?

See #27094.

@poutsma
Copy link
Contributor

poutsma commented Jun 24, 2021

I only adjusted a few lines of code. If necessary, I can submit a PR.

I have made a similar change myself, see above, so a PR won't be necessary. Thank you though.

Note that we have stopped using Synchronoss as default multiparty parser as of version 5.3, where we ship our own DefaultPartHttpMessageReader, which only creates temp directories when the size exceeds a defined limit.

@RekaDowney
Copy link

Note that we have stopped using Synchronoss as default multiparty parser as of version 5.3, where we ship our own DefaultPartHttpMessageReader, which only creates temp directories when the size exceeds a defined limit.

Good, We will add the upgrade plan to our schedule.

Zoran0104 pushed a commit to Zoran0104/spring-framework that referenced this issue Aug 20, 2021
The SynchronossPartHttpMessageReader should only create temp directory
when needed, not at startup.

Closes spring-projectsgh-27092
lxbzmy pushed a commit to lxbzmy/spring-framework that referenced this issue Mar 26, 2022
The SynchronossPartHttpMessageReader should only create temp directory
when needed, not at startup.

Closes spring-projectsgh-27092
duhanmin added a commit to duhanmin/incubator-linkis that referenced this issue Jul 1, 2022
spring-projects/spring-framework#27092
1 . SynchronossPartHttpMessageReader should only create temp directory when needed 
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-22965
2. A Spring MVC or Spring WebFlux application running on JDK 9+ may be vulnerable to remote code execution (RCE) via data binding. The specific exploit requires the application to run on Tomcat as a WAR deployment. If the application is deployed as a Spring Boot executable jar, i.e. the default, it is not vulnerable to the exploit. However, the nature of the vulnerability is more general, and there may be other ways to exploit it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants