Skip to content

Commit

Permalink
GH-9711: Avoid double URL encoding in SmbShare
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
artembilan authored and spring-builds committed Dec 17, 2024
1 parent 04845b0 commit ef3ee8f
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 65 deletions.
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -25,7 +25,7 @@
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[...]]]
Expand Down Expand Up @@ -197,22 +197,50 @@ public final String getUrl() {
}

public final String getUrl(boolean _includePassword) {
String domainUserPass = getDomainUserPass(_includePassword);
return createUri(_includePassword).toASCIIString();
}

String path = StringUtils.cleanPath(this.shareAndDir);
/**
* 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);
}

if (!path.startsWith("/")) {
path = "/" + path;
}
/**
* 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)
.toASCIIString();
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;
}
return path;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())));
Expand All @@ -76,7 +76,7 @@ public SmbShare(SmbConfig _smbConfig) throws IOException {
* @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);
}

/**
Expand All @@ -88,7 +88,7 @@ public SmbShare(SmbConfig _smbConfig, CIFSContext _context) throws IOException {
* @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(
Expand Down
Original file line number Diff line number Diff line change
@@ -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>

Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -47,8 +47,10 @@ public void testMessageHistory() throws URISyntaxException {

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/");
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,4 @@ public void cleanUp() {
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");
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -110,7 +110,7 @@ public static void connectToSMBServer() throws IOException {
}

public static String smbServerUrl() {
return smbSessionFactory.getUrl().replaceFirst('/' + SHARE_AND_DIR + '/', "");
return smbSessionFactory.rawUrl().replaceFirst('/' + SHARE_AND_DIR + '/', "");
}

public static SessionFactory<SmbFile> sessionFactory() {
Expand Down

0 comments on commit ef3ee8f

Please sign in to comment.