From 5e3d8d890a547c6b881c4f5c3c9d153d25b35eb9 Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Tue, 17 Dec 2024 13:06:46 -0500 Subject: [PATCH] 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 **Auto-cherry-pick to `6.3.x`** --- .../integration/smb/session/SmbConfig.java | 46 +++++++++++++--- .../integration/smb/session/SmbShare.java | 6 +- .../smb/SmbMessageHistoryTests-context.xml | 33 +++++------ .../smb/SmbMessageHistoryTests.java | 8 ++- .../smb/SmbParserInboundTests-context.xml | 55 ++++++++++--------- .../smb/SmbParserInboundTests.java | 5 -- .../integration/smb/SmbTestSupport.java | 4 +- 7 files changed, 92 insertions(+), 65 deletions(-) diff --git a/spring-integration-smb/src/main/java/org/springframework/integration/smb/session/SmbConfig.java b/spring-integration-smb/src/main/java/org/springframework/integration/smb/session/SmbConfig.java index 27de4cef6cc..22fe581ae4f 100644 --- a/spring-integration-smb/src/main/java/org/springframework/integration/smb/session/SmbConfig.java +++ b/spring-integration-smb/src/main/java/org/springframework/integration/smb/session/SmbConfig.java @@ -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.StringUtils; /** - * Data holder class for a SMB share configuration. + * Data holder class for an SMB share configuration. *

* SmbFile URLs syntax: * smb://[[[domain;]username[:password]@]server[:port]/[[share/[dir/]file]]][?[param=value[param2=value2[...]]] @@ -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 diff --git a/spring-integration-smb/src/main/java/org/springframework/integration/smb/session/SmbShare.java b/spring-integration-smb/src/main/java/org/springframework/integration/smb/session/SmbShare.java index d287a481e19..f96102937e2 100644 --- a/spring-integration-smb/src/main/java/org/springframework/integration/smb/session/SmbShare.java +++ b/spring-integration-smb/src/main/java/org/springframework/integration/smb/session/SmbShare.java @@ -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 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); } /** @@ -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( diff --git a/spring-integration-smb/src/test/java/org/springframework/integration/smb/SmbMessageHistoryTests-context.xml b/spring-integration-smb/src/test/java/org/springframework/integration/smb/SmbMessageHistoryTests-context.xml index 9c9bf00a913..ecda2bf3478 100644 --- a/spring-integration-smb/src/test/java/org/springframework/integration/smb/SmbMessageHistoryTests-context.xml +++ b/spring-integration-smb/src/test/java/org/springframework/integration/smb/SmbMessageHistoryTests-context.xml @@ -1,30 +1,31 @@ - - - - - - + + + + + + + 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"> diff --git a/spring-integration-smb/src/test/java/org/springframework/integration/smb/SmbMessageHistoryTests.java b/spring-integration-smb/src/test/java/org/springframework/integration/smb/SmbMessageHistoryTests.java index 643fbe3cd7e..65549313e39 100644 --- a/spring-integration-smb/src/test/java/org/springframework/integration/smb/SmbMessageHistoryTests.java +++ b/spring-integration-smb/src/test/java/org/springframework/integration/smb/SmbMessageHistoryTests.java @@ -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 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/"); } } diff --git a/spring-integration-smb/src/test/java/org/springframework/integration/smb/SmbParserInboundTests-context.xml b/spring-integration-smb/src/test/java/org/springframework/integration/smb/SmbParserInboundTests-context.xml index a3bced155cd..88483c27ac1 100644 --- a/spring-integration-smb/src/test/java/org/springframework/integration/smb/SmbParserInboundTests-context.xml +++ b/spring-integration-smb/src/test/java/org/springframework/integration/smb/SmbParserInboundTests-context.xml @@ -1,48 +1,49 @@ - - - - + + + + - + 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"> + - + 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"> + - + - + diff --git a/spring-integration-smb/src/test/java/org/springframework/integration/smb/SmbParserInboundTests.java b/spring-integration-smb/src/test/java/org/springframework/integration/smb/SmbParserInboundTests.java index 5642a2b74fe..a29ee0b6f8c 100644 --- a/spring-integration-smb/src/test/java/org/springframework/integration/smb/SmbParserInboundTests.java +++ b/spring-integration-smb/src/test/java/org/springframework/integration/smb/SmbParserInboundTests.java @@ -63,9 +63,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"); - } - } diff --git a/spring-integration-smb/src/test/java/org/springframework/integration/smb/SmbTestSupport.java b/spring-integration-smb/src/test/java/org/springframework/integration/smb/SmbTestSupport.java index a641226173e..b8020972572 100644 --- a/spring-integration-smb/src/test/java/org/springframework/integration/smb/SmbTestSupport.java +++ b/spring-integration-smb/src/test/java/org/springframework/integration/smb/SmbTestSupport.java @@ -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 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 sessionFactory() {