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

SMB components fail to access files with spaces in its path due to duplicate URL encoding #9711

Closed
smitsjelle opened this issue Dec 12, 2024 · 1 comment

Comments

@smitsjelle
Copy link

In what version(s) of Spring Integration are you seeing this issue?
6.4.0 & 6.2.1 (likely affects older versions too, but tested on these)

Describe the bug

When attempting to connect to an SMB share where either the 'share and dir', the 'remote directory' or the file name contains a whitespace, this will be URL-encoded to %20. It appears that the URL encoding is applied twice, both by Spring integration as well as by the underlying codelibs/jcifs library.

Essentially, in SmbShare, Spring calls the constructor of its parent jcifs.smb.SmbFile with the String url argument (sidenote: this contructor appears to be deprecated). To obtain the URL string, SmbConfig#209 creates an URI from it, an returns the ASCIIString that maintains the URL encoding. The parent class subsequently creates an URL object out of this already URL-encoded String.

This appears to cause double encoding, resulting in e.g. an outbound channel adapter with auto create directory true creating the directory space%20directory instead of the provided argument space directory

To Reproduce

Using the existing Spring integration unit tests, this can easiliy be reproduced. In SmbTestSupport, change SHARE_AND_DIR to smb share, and error will occur. Subsequently change the output of SmbConfig#toUrl() to a non-url-encoded String (e.g. by replacing line 209 with return "smb://%s@%s/%s".formatted(domainUserPass, getHostPort(), path); and it will succeed.

Though the line above seems to solve it for this specific case, I have not tested thoroughly on other edge cases, and I am not sure whether there are valid considerations why we would still need to use the URI.toAsciiString call for other reasons. I feel a part of it may already be handled as the call to StringUtils.cleanPath().

Expected behavior

Even though I'd like to be able to 'fix' the underlying directories so they do not contain whitespaces, I'd like to be able to access those directories, as it appears to be supported by the libarary and now just fails because double URL encoding.

@smitsjelle smitsjelle added status: waiting-for-triage The issue need to be evaluated and its future decided type: bug labels Dec 12, 2024
@artembilan artembilan added this to the 6.4.1 milestone Dec 12, 2024
@artembilan artembilan added in: smb for: backport-to-6.3.x and removed status: waiting-for-triage The issue need to be evaluated and its future decided labels Dec 12, 2024
@artembilan
Copy link
Member

I think your investigation is correct: we must not set an encoded string into an URL constructor.

Please, consider to contribute the fix: https://github.com/spring-projects/spring-integration/blob/main/CONTRIBUTING.adoc

spring-builds pushed a commit that referenced this issue Dec 17, 2024
Fixes: #9711
Issue link: #9711

The `SmbConfig.getUrl(_includePassword)` uses `URI.toASCIIString()` which has all the parts of the URI encoded.
Then this string is used by the `SmbShare` for its super constructor, which, in turn, calls `new URL()`
leading, essentially, to a second encoding round.

* Add `SmbConfig.rawUrl()` methods to return an `smb://` url as a plain string.
* Use this new API in the `SmbShare` for a delegation constructor
* Modify some tests to reflect and cover new behavior

(cherry picked from commit 5e3d8d8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants