Skip to content

GH-3211: Add DefSftpSF.setKnownHosts(Resource) #3212

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.

Sign up for GitHub

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

Merged
garyrussell merged 2 commits into from
Mar 10, 2020
Merged

GH-3211: Add DefSftpSF.setKnownHosts(Resource) #3212

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
02451c1
GH-3211: Add DefSftpSF.setKnownHosts(Resource)
artembilan Mar 10, 2020
7de122d
* Rename to `setKnownHostsResource()` to avoid XML parser confusion
artembilan Mar 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 24 additions & 9 deletions ...src/main/java/org/springframework/integration/sftp/session/DefaultSftpSessionFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 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 @@ -17,6 +17,7 @@
package org.springframework.integration.sftp.session;

import java.io.IOException;
import java.io.InputStream;
import java.time.Duration;
import java.util.Arrays;
import java.util.Properties;
Expand All @@ -27,6 +28,7 @@
import org.apache.commons.logging.LogFactory;

import org.springframework.beans.factory.BeanCreationException;
import org.springframework.core.io.FileSystemResource;
import org.springframework.core.io.Resource;
import org.springframework.integration.file.remote.session.SessionFactory;
import org.springframework.integration.file.remote.session.SharedSessionCapable;
Expand Down Expand Up @@ -81,7 +83,7 @@ public class DefaultSftpSessionFactory implements SessionFactory<LsEntry>, Share

private String password;

private String knownHosts;
private Resource knownHosts;

private Resource privateKey;

Expand Down Expand Up @@ -191,8 +193,21 @@ public void setPassword(String password) {
* false (default).</b>
* @param knownHosts The known hosts.
* @see JSch#setKnownHosts(String)
* @deprecated since 5.2.5 in favor of {@link #setKnownHosts(Resource)}
*/
@Deprecated
Copy link
Contributor

@garyrussell garyrussell Mar 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we really deprecate? This is easier if you are using Java Config and want to point to a simple file name.

Not related to this PR but perhaps we should consider adding a SftpSessionFactorySpec ?

Copy link
Member Author

@artembilan artembilan Mar 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we need also to add a string-based for privateKey.

More over it is better to resolve any resources via @Value Resource then inject.

public void setKnownHosts(String knownHosts) {
setKnownHosts(new FileSystemResource(knownHosts));
}

/**
* Specifies the filename that will be used for a host key repository.
* The file has the same format as OpenSSH's known_hosts file.
* @param knownHosts the resource for known hosts.
* @see JSch#setKnownHosts(InputStream)
* @since 5.2.5
*/
public void setKnownHosts(Resource knownHosts) {
Copy link
Contributor

@garyrussell garyrussell Mar 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this cause a problem with <bean/> config? (ambiguous setter) setKnownHostsResource() ?

Copy link
Member Author

@artembilan artembilan Mar 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like Juergen has fixed something in SF. See Travis 😄

Copy link
Contributor

@garyrussell garyrussell Mar 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I recall correctly, failures were random; also it looks like you changed all the tests to return a Resource using SpEL.

Perhaps the problem only occurs when a PropertyEditor is needed (although I remember most of the problems were with String Vs. Expression .

I will defer to you, though, if you think it's safe.

Copy link
Member Author

@artembilan artembilan Mar 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I can rename it to setKnownHostsResource to have it really very safe...

this.knownHosts = knownHosts;
}

Expand Down Expand Up @@ -323,7 +338,7 @@ public void setEnableDaemonThread(Boolean enableDaemonThread) {
* implementation must respond to Jsch calls in a suitable way.
* <p>
* Jsch calls {@link UserInfo#promptYesNo(String)} when connecting to an unknown host,
* or when a known host's key has changed (see {@link #setKnownHosts(String)
* or when a known host's key has changed (see {@link #setKnownHosts(Resource)}
* knownHosts}). Generally, it should return false as returning true will accept all
* new keys or key changes.
* <p>
Expand All @@ -347,7 +362,7 @@ public void setUserInfo(UserInfo userInfo) {
/**
* When no {@link UserInfo} has been provided, set to true to unconditionally allow
* connecting to an unknown host or when a host's key has changed (see
* {@link #setKnownHosts(String) knownHosts}). Default false (since 4.2).
* {@link #setKnownHosts(Resource) knownHosts}). Default false (since 4.2).
* Set to true if a knownHosts file is not provided.
* @param allowUnknownKeys true to allow connecting to unknown hosts.
* @since 4.1.7
Expand Down Expand Up @@ -380,8 +395,7 @@ public SftpSession getSession() {
freshJschSession = true;
}
sftpSession = new SftpSession(jschSession);
JavaUtils.INSTANCE
.acceptIfNotNull(this.channelConnectTimeout, sftpSession::setChannelConnectTimeout);
JavaUtils.INSTANCE.acceptIfNotNull(this.channelConnectTimeout, sftpSession::setChannelConnectTimeout);
sftpSession.connect();
if (this.isSharedSession && freshJschSession) {
this.sharedJschSession = jschSession;
Expand All @@ -408,8 +422,8 @@ private com.jcraft.jsch.Session initJschSession() throws JSchException, IOExcept
if (this.port <= 0) {
this.port = 22;
}
if (StringUtils.hasText(this.knownHosts)) {
this.jsch.setKnownHosts(this.knownHosts);
if (this.knownHosts != null) {
this.jsch.setKnownHosts(this.knownHosts.getInputStream());
}

// private key
Expand Down Expand Up @@ -467,6 +481,7 @@ public void resetSharedSession() {
* sensible defaults if null. As the password is configured in this Factory, the
* wrapper will return the factory's configured password and only delegate to the
* UserInfo if null.
*
* @since 4.1.7
*/
private class UserInfoWrapper implements UserInfo, UIKeyboardInteractive {
Expand Down Expand Up @@ -548,7 +563,7 @@ public boolean promptYesNo(String message) {
}
else {
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("No UserInfo provided - " + message + ", returning:"
LOGGER.debug("No UserInfo provided - " + message + ", returning: "
+ DefaultSftpSessionFactory.this.allowUnknownKeys);
}
return DefaultSftpSessionFactory.this.allowUnknownKeys;
Expand Down
2 changes: 1 addition & 1 deletion ...ringframework/integration/sftp/config/InboundChannelAdapterParserCachingTests-context.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

<beans:bean id="sftpSessionFactory" class="org.springframework.integration.sftp.session.DefaultSftpSessionFactory">
<beans:property name="host" value="loclahost"/>
<beans:property name="knownHosts" value="local, foo.com, bar.foo"/>
<beans:property name="knownHosts" value="#{ new org.springframework.core.io.ByteArrayResource('local, foo.com, bar.foo'.bytes)}"/>
<beans:property name="privateKey" value="classpath:org/springframework/integration/sftp/config/sftp_rsa"/>
<beans:property name="privateKeyPassphrase" value="ghj"/>
<beans:property name="password" value="hello"/>
Expand Down
28 changes: 10 additions & 18 deletions ...work/integration/sftp/config/InboundChannelAdapterParserTests-context-fail-autocreate.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,17 @@
xmlns="http://www.springframework.org/schema/integration"
xmlns:beans="http://www.springframework.org/schema/beans"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:p="http://www.springframework.org/schema/p"
xmlns:context="http://www.springframework.org/schema/context"
xmlns:util="http://www.springframework.org/schema/util"
xmlns:tool="http://www.springframework.org/schema/tool"
xmlns:lang="http://www.springframework.org/schema/lang"
xmlns:sftp="http://www.springframework.org/schema/integration/sftp"
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/context https://www.springframework.org/schema/context/spring-context.xsd
http://www.springframework.org/schema/util https://www.springframework.org/schema/util/spring-util.xsd
http://www.springframework.org/schema/tool https://www.springframework.org/schema/tool/spring-tool.xsd
http://www.springframework.org/schema/lang https://www.springframework.org/schema/lang/spring-lang.xsd
http://www.springframework.org/schema/integration/sftp https://www.springframework.org/schema/integration/sftp/spring-integration-sftp.xsd">

<channel id="requestChannel"/>

<beans:bean id="sftpSessionFactory" class="org.springframework.integration.sftp.session.DefaultSftpSessionFactory">
<beans:property name="host" value="loclahost"/>
<beans:property name="knownHosts" value="local, foo.com, bar.foo"/>
<beans:property name="knownHosts"
value="#{ new org.springframework.core.io.ByteArrayResource('local, foo.com, bar.foo'.bytes)}"/>
<beans:property name="privateKey" value="classpath:org/springframework/integration/sftp/config/sftpTest"/>
<beans:property name="privateKeyPassphrase" value="ghj"/>
<beans:property name="password" value="hello"/>
Expand All @@ -30,17 +22,17 @@
</beans:bean>

<sftp:inbound-channel-adapter id="sftpAdapterNoAutoCreate"
channel="requestChannel"
session-factory="sftpSessionFactory"
filter="filter"
remote-directory="/foo"
local-directory="file:foo"
auto-create-local-directory="false"
delete-remote-files="false">
channel="requestChannel"
session-factory="sftpSessionFactory"
filter="filter"
remote-directory="/foo"
local-directory="file:foo"
auto-create-local-directory="false"
delete-remote-files="false">
<poller fixed-rate="1000"/>
</sftp:inbound-channel-adapter>

<beans:bean id="filter" class="org.springframework.integration.sftp.filters.SftpPatternMatchingFileListFilter">
<beans:bean id="filter" class="org.springframework.integration.sftp.filters.SftpSimplePatternFileListFilter">
<beans:constructor-arg value="."/>
</beans:bean>

Expand Down
3 changes: 2 additions & 1 deletion .../org/springframework/integration/sftp/config/InboundChannelAdapterParserTests-context.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@

<beans:bean id="sftpSessionFactory" class="org.springframework.integration.sftp.session.DefaultSftpSessionFactory">
<beans:property name="host" value="loclahost"/>
<beans:property name="knownHosts" value="local, foo.com, bar.foo"/>
<beans:property name="knownHosts"
value="#{ new org.springframework.core.io.ByteArrayResource('local, foo.com, bar.foo'.bytes)}"/>
<beans:property name="privateKey" value="classpath:org/springframework/integration/sftp/config/sftp_rsa"/>
<beans:property name="privateKeyPassphrase" value="ghj"/>
<beans:property name="password" value="hello"/>
Expand Down
12 changes: 2 additions & 10 deletions ...sftp/src/test/java/org/springframework/integration/sftp/config/MessageHistory-context.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,17 @@
xmlns="http://www.springframework.org/schema/integration"
xmlns:beans="http://www.springframework.org/schema/beans"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:p="http://www.springframework.org/schema/p"
xmlns:context="http://www.springframework.org/schema/context"
xmlns:util="http://www.springframework.org/schema/util"
xmlns:tool="http://www.springframework.org/schema/tool"
xmlns:lang="http://www.springframework.org/schema/lang"
xmlns:sftp="http://www.springframework.org/schema/integration/sftp"
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/context https://www.springframework.org/schema/context/spring-context.xsd
http://www.springframework.org/schema/util https://www.springframework.org/schema/util/spring-util.xsd
http://www.springframework.org/schema/tool https://www.springframework.org/schema/tool/spring-tool.xsd
http://www.springframework.org/schema/lang https://www.springframework.org/schema/lang/spring-lang.xsd
http://www.springframework.org/schema/integration/sftp https://www.springframework.org/schema/integration/sftp/spring-integration-sftp.xsd">

<channel id="inboundFilesChannel"/>

<beans:bean id="sftpSessionFactory" class="org.springframework.integration.sftp.session.DefaultSftpSessionFactory">
<beans:property name="host" value="loclahost"/>
<beans:property name="knownHosts" value="local, foo.com, bar.foo"/>
<beans:property name="knownHosts"
value="#{ new org.springframework.core.io.ByteArrayResource('local, foo.com, bar.foo'.bytes)}"/>
<beans:property name="privateKey" value="classpath:org/springframework/integration/sftp/config/sftpTest"/>
<beans:property name="privateKeyPassphrase" value="ghj"/>
<beans:property name="password" value="hello"/>
Expand Down
3 changes: 2 additions & 1 deletion ...ingframework/integration/sftp/config/OutboundChannelAdapterParserCachingTests-context.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@

<bean id="sftpSessionFactory" class="org.springframework.integration.sftp.session.DefaultSftpSessionFactory">
<property name="host" value="localhost"/>
<property name="knownHosts" value="local, foo.com, bar.foo"/>
<property name="knownHosts"
value="#{ new org.springframework.core.io.ByteArrayResource('local, foo.com, bar.foo'.bytes)}"/>
<property name="privateKey" value="classpath:org/springframework/integration/sftp/config/sftpTest"/>
<property name="privateKeyPassphrase" value="ghj"/>
<property name="password" value="hello"/>
Expand Down
3 changes: 2 additions & 1 deletion ...pringframework/integration/sftp/config/OutboundChannelAdapterParserTests-context-fail.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@

<bean id="sftpSessionFactory" class="org.springframework.integration.sftp.session.DefaultSftpSessionFactory">
<property name="host" value="localhost"/>
<property name="knownHosts" value="local, foo.com, bar.foo"/>
<property name="knownHosts"
value="#{ new org.springframework.core.io.ByteArrayResource('local, foo.com, bar.foo'.bytes)}"/>
<property name="privateKey" value="classpath:org/springframework/integration/sftp/config/sftpTest"/>
<property name="privateKeyPassphrase" value="ghj"/>
<property name="password" value="hello"/>
Expand Down
3 changes: 2 additions & 1 deletion ...org/springframework/integration/sftp/config/OutboundChannelAdapterParserTests-context.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@

<bean id="sftpSessionFactory" class="org.springframework.integration.sftp.session.DefaultSftpSessionFactory">
<property name="host" value="localhost"/>
<property name="knownHosts" value="local, foo.com, bar.foo"/>
<property name="knownHosts"
value="#{ new org.springframework.core.io.ByteArrayResource('local, foo.com, bar.foo'.bytes)}"/>
<property name="privateKey" value="classpath:org/springframework/integration/sftp/config/sftpTest"/>
<property name="privateKeyPassphrase" value="ghj"/>
<property name="password" value="hello"/>
Expand Down
3 changes: 2 additions & 1 deletion .../test/java/org/springframework/integration/sftp/config/SftpInboundAutostartup-context.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@

<bean id="sftpSessionFactory" class="org.springframework.integration.sftp.session.DefaultSftpSessionFactory">
<property name="host" value="loclahost"/>
<property name="knownHosts" value="local, foo.com, bar.foo"/>
<property name="knownHosts"
value="#{ new org.springframework.core.io.ByteArrayResource('local, foo.com, bar.foo'.bytes)}"/>
<property name="privateKey" value="classpath:org/springframework/integration/sftp/config/sftp_rsa"/>
<property name="privateKeyPassphrase" value="ghj"/>
<property name="password" value="hello"/>
Expand Down