GH-9711: Avoid double URL encoding in SmbShare

Fixes: #9711
Issue link: https://github.com/spring-projects/spring-integration/issues/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 5e3d8d890a)
This commit is contained in:
Artem Bilan
2024-12-17 13:06:46 -05:00
committed by Spring Builds
parent 04845b0561
commit ef3ee8f427
7 changed files with 95 additions and 68 deletions

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2012-2022 the original author or authors.
* Copyright 2012-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -25,7 +25,7 @@ import org.springframework.util.Assert;
import org.springframework.util.StringUtils;
/**
* Data holder class for a SMB share configuration.
* Data holder class for an SMB share configuration.
*<p>
* SmbFile URLs syntax:
* smb://[[[domain;]username[:password]@]server[:port]/[[share/[dir/]file]]][?[param=value[param2=value2[...]]]
@@ -197,22 +197,50 @@ public class SmbConfig {
}
public final String getUrl(boolean _includePassword) {
String domainUserPass = getDomainUserPass(_includePassword);
return createUri(_includePassword).toASCIIString();
}
/**
* Return the url string for the share connection without encoding.
* Used in the {@link SmbShare} constructor delegation.
* @return the url string for the share connection without encoding.
* @since 6.3.8
*/
public final String rawUrl() {
return rawUrl(true);
}
/**
* Return the url string for the share connection without encoding.
* Used in the {@link SmbShare} constructor delegation.
* @param _includePassword whether password has to be masked in credentials of URL.
* @return the url string for the share connection without encoding.
* @since 6.3.8
*/
public final String rawUrl(boolean _includePassword) {
String domainUserPass = getDomainUserPass(_includePassword);
String path = cleanPath();
return "smb://%s@%s%s".formatted(domainUserPass, getHostPort(), path);
}
private URI createUri(boolean _includePassword) {
String domainUserPass = getDomainUserPass(_includePassword);
String path = cleanPath();
try {
return new URI("smb", domainUserPass, this.host, this.port, path, null, null);
}
catch (URISyntaxException e) {
throw new IllegalArgumentException(e);
}
}
private String cleanPath() {
String path = StringUtils.cleanPath(this.shareAndDir);
if (!path.startsWith("/")) {
path = "/" + path;
}
try {
return new URI("smb", domainUserPass, this.host, this.port, path, null, null)
.toASCIIString();
}
catch (URISyntaxException e) {
throw new IllegalArgumentException(e);
}
return path;
}
@Override

View File

@@ -63,7 +63,7 @@ public class SmbShare extends SmbFile {
* @throws IOException if an invalid SMB URL was constructed by jCIFS
*/
public SmbShare(SmbConfig _smbConfig) throws IOException {
super(StringUtils.cleanPath(_smbConfig.validate().getUrl()),
super(StringUtils.cleanPath(_smbConfig.validate().rawUrl()),
SingletonContext.getInstance().withCredentials(
new NtlmPasswordAuthenticator(
_smbConfig.getDomain(), _smbConfig.getUsername(), _smbConfig.getPassword())));
@@ -76,7 +76,7 @@ public class SmbShare extends SmbFile {
* @throws IOException if an invalid SMB URL was constructed by jCIFS
*/
public SmbShare(SmbConfig _smbConfig, CIFSContext _context) throws IOException {
super(StringUtils.cleanPath(_smbConfig.validate().getUrl()), _context);
super(StringUtils.cleanPath(_smbConfig.validate().rawUrl()), _context);
}
/**
@@ -88,7 +88,7 @@ public class SmbShare extends SmbFile {
* @throws IOException if an invalid property was set or an invalid SMB URL was constructed by jCIFS
*/
public SmbShare(SmbConfig _smbConfig, Properties _props) throws IOException {
super(StringUtils.cleanPath(_smbConfig.validate().getUrl()),
super(StringUtils.cleanPath(_smbConfig.validate().rawUrl()),
new BaseContext(
new PropertyConfiguration(_props)).withCredentials(
new NtlmPasswordAuthenticator(

View File

@@ -1,30 +1,31 @@
<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:int="http://www.springframework.org/schema/integration"
xmlns:int-smb="http://www.springframework.org/schema/integration/smb"
xsi:schemaLocation="http://www.springframework.org/schema/beans https://www.springframework.org/schema/beans/spring-beans.xsd
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:int="http://www.springframework.org/schema/integration"
xmlns:int-smb="http://www.springframework.org/schema/integration/smb"
xsi:schemaLocation="http://www.springframework.org/schema/beans https://www.springframework.org/schema/beans/spring-beans.xsd
http://www.springframework.org/schema/integration https://www.springframework.org/schema/integration/spring-integration.xsd
http://www.springframework.org/schema/integration/smb https://www.springframework.org/schema/integration/smb/spring-integration-smb.xsd">
<int:message-history/>
<bean id="smbSessionFactory" class="org.springframework.integration.smb.session.SmbSessionFactory">
<property name="host" value="localhost"/>
<property name="port" value="0"/>
<property name="domain" value=""/>
<property name="username" value="sambagu@est"/>
<property name="password" value="sambag%uest"/>
<property name="shareAndDir" value="smb-share/"/>
<property name="host" value="localhost"/>
<property name="port" value="0"/>
<property name="domain" value=""/>
<property name="username" value="sambagu@est"/>
<property name="password" value="sambag%uest"/>
<property name="shareAndDir" value="smb share/"/>
</bean>
<int-smb:inbound-channel-adapter id="smbInboundChannelAdapter"
session-factory="smbSessionFactory"
channel="smbInboundChannel"
auto-create-local-directory="true"
local-directory="file:test-temp/local-5"
remote-directory="test-temp/remote-9"
delete-remote-files="false">
session-factory="smbSessionFactory"
channel="smbInboundChannel"
auto-create-local-directory="true"
local-directory="file:test-temp/local-5"
remote-directory="test-temp/remote-9"
auto-startup="false"
delete-remote-files="false">
<int:poller fixed-rate="1000"/>
</int-smb:inbound-channel-adapter>

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2012-2023 the original author or authors.
* Copyright 2012-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -47,8 +47,10 @@ public class SmbMessageHistoryTests extends AbstractBaseTests {
String url = smbSessionFactory.getUrl();
URI uri = new URI(url);
assertThat("sambagu%40est:sambag%25uest").isEqualTo(uri.getRawUserInfo());
assertThat("sambagu@est:sambag%uest").isEqualTo(uri.getUserInfo());
assertThat(uri.getRawUserInfo()).isEqualTo("sambagu%40est:sambag%25uest");
assertThat(uri.getUserInfo()).isEqualTo("sambagu@est:sambag%uest");
assertThat(uri.getPath()).isEqualTo("/smb share/");
assertThat(uri.getRawPath()).isEqualTo("/smb%20share/");
}
}

View File

@@ -1,48 +1,49 @@
<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:int="http://www.springframework.org/schema/integration"
xmlns:int-smb="http://www.springframework.org/schema/integration/smb"
xmlns:context="http://www.springframework.org/schema/context"
xsi:schemaLocation="http://www.springframework.org/schema/beans https://www.springframework.org/schema/beans/spring-beans.xsd
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:int="http://www.springframework.org/schema/integration"
xmlns:int-smb="http://www.springframework.org/schema/integration/smb"
xsi:schemaLocation="http://www.springframework.org/schema/beans https://www.springframework.org/schema/beans/spring-beans.xsd
http://www.springframework.org/schema/integration https://www.springframework.org/schema/integration/spring-integration.xsd
http://www.springframework.org/schema/integration/smb https://www.springframework.org/schema/integration/smb/spring-integration-smb.xsd">
<bean id="smbSessionFactory" class="org.springframework.integration.smb.session.SmbSessionFactory">
<property name="host" value="localhost"/>
<property name="domain" value=""/>
<property name="username" value="sambaguest"/>
<property name="password" value="sambaguest"/>
<property name="host" value="localhost"/>
<property name="domain" value=""/>
<property name="username" value="sambaguest"/>
<property name="password" value="sambaguest"/>
<property name="shareAndDir" value="smb-share/"/>
</bean>
<int-smb:inbound-channel-adapter id="adapterSmb"
session-factory="smbSessionFactory"
channel="smbIn"
filename-pattern="foo"
local-directory="test-temp/local-10"
remote-directory="test-temp/remote-10"
auto-create-local-directory="true"
delete-remote-files="false">
<int:poller ref="smbPoller" />
session-factory="smbSessionFactory"
channel="smbIn"
filename-pattern="foo"
local-directory="test-temp/local-10"
remote-directory="test-temp/remote-10"
auto-create-local-directory="true"
auto-startup="false"
delete-remote-files="false">
<int:poller ref="smbPoller"/>
</int-smb:inbound-channel-adapter>
<int-smb:inbound-channel-adapter id="adapterSmb2"
session-factory="smbSessionFactory"
channel="smbIn"
filter="filter"
local-directory="test-temp"
remote-directory="test-temp/remote-11"
auto-create-local-directory="true"
delete-remote-files="false">
<int:poller ref="smbPoller" />
session-factory="smbSessionFactory"
channel="smbIn"
filter="filter"
local-directory="test-temp"
remote-directory="test-temp/remote-11"
auto-create-local-directory="true"
auto-startup="false"
delete-remote-files="false">
<int:poller ref="smbPoller"/>
</int-smb:inbound-channel-adapter>
<bean id="filter" class="org.mockito.Mockito" factory-method="mock">
<constructor-arg value="org.springframework.integration.file.filters.FileListFilter" type="java.lang.Class"/>
<constructor-arg value="org.springframework.integration.file.filters.FileListFilter" type="java.lang.Class"/>
</bean>
<int:poller fixed-rate="3000" id="smbPoller" />
<int:poller fixed-rate="3000" id="smbPoller"/>
<int:channel id="smbIn">
<int:queue/>

View File

@@ -60,9 +60,4 @@ public class SmbParserInboundTests extends AbstractBaseTests {
delete("test-temp/local-10", "test-temp/local-6");
}
public static void main(String[] _args) throws Exception {
new SmbParserInboundTests().cleanUp();
runTests(SmbParserInboundTests.class, "testLocalFilesAutoCreationTrue", "testLocalFilesAutoCreationFalse");
}
}

View File

@@ -68,7 +68,7 @@ public class SmbTestSupport extends RemoteFileTestSupport {
public static final String HOST = "127.0.0.1";
public static final String SHARE_AND_DIR = "smb-share";
public static final String SHARE_AND_DIR = "smb share";
public static final String USERNAME = "sambaguest";
@@ -110,7 +110,7 @@ public class SmbTestSupport extends RemoteFileTestSupport {
}
public static String smbServerUrl() {
return smbSessionFactory.getUrl().replaceFirst('/' + SHARE_AND_DIR + '/', "");
return smbSessionFactory.rawUrl().replaceFirst('/' + SHARE_AND_DIR + '/', "");
}
public static SessionFactory<SmbFile> sessionFactory() {