Skip to content

Inconsistent behaviour between listing and downloading the files. #9123

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

Closed
SimoneGiusso opened this issue May 6, 2024 · 11 comments
Closed

Comments

@SimoneGiusso
Copy link

SimoneGiusso commented May 6, 2024

In what version(s) of Spring Integration are you seeing this issue?

6.2.4

Describe the bug

/ at the beginning of the remote dir path is not necessary when listing files although it is necessary to download them

To Reproduce

    @Test
    fun testFileDownload() {
        sftpTemplate.list("remote-dir").forEach { // All the files under remote-dir are listed
            log.info { it.filename }
        }

        log.info { "---- FINISHED TO LIST DIRECTORIES -----\n\n\n\n" }

        sftpTemplate.get("/remote-dir/MyFile.csv") { // remote-dir/MyFile.csv won't work
            inputStream -> inputStream.use {
                localDir.mkdir()
                FileUtils.copyInputStreamToFile(inputStream, File(localDir.absolutePath + "test.csv"))
        }}
    }

Expected behavior

sftpTemplate.get should work also with "remote-dir/MyFile.csv" as input.

See also stackOverflow question.

@SimoneGiusso SimoneGiusso added status: waiting-for-triage The issue need to be evaluated and its future decided type: bug labels May 6, 2024
@SimoneGiusso SimoneGiusso changed the title Inconsistent behaviour when between listing and downloading the files. Inconsistent behaviour between listing and downloading the files. May 6, 2024
@artembilan
Copy link
Member

For LIST command we do this:

		remoteDir =
				!remoteDir.isEmpty() && remoteDir.charAt(0) == '/'
						? remoteDir
						: this.sftpClient.canonicalPath(remoteDir);

For GET - do not:

	public InputStream readRaw(String source) throws IOException {
		return this.sftpClient.read(source);
	}

So, maybe this is a problem and we can try to normalize path if it does not start with /?

@SimoneGiusso
Copy link
Author

Yes, both commands should threat the dirPath in the same way.

Here probably the GET should normalize the path as the LIST command does.

I found unexpected the fact that one command without "/" does not work while it works for the other.

@SimoneGiusso
Copy link
Author

Additionally, in the message's header file_remoteDirectory, two '/' are present before the remote repository name

@artembilan
Copy link
Member

@SimoneGiusso , do you mean for the SftpOutboundGateway with a Command.GET ?
Or what part of the flow you are talking about which produces the Message for you?

@artembilan artembilan added in: sftp and removed status: waiting-for-triage The issue need to be evaluated and its future decided labels May 7, 2024
@artembilan artembilan added this to the 6.3.0 milestone May 7, 2024
spring-builds pushed a commit that referenced this issue May 7, 2024
Fixes: #9123

The `/` at the beginning of the remote dir path is not necessary when listing files, although it is necessary to download them
The `SftpTemplate.get()` should work also with `remote-dir/MyFile.csv` as input.

* Fix `SftpSession.readRaw()` to call `sftpClient.canonicalPath(source)` if the path does not start with a `/`.
Something similar what is does
* Delegate to `SftpSession.readRaw()` from the `SftpSession.read()`
* Reuse `normalizePath()` for `doList()`

(cherry picked from commit a2215af)
spring-builds pushed a commit that referenced this issue May 7, 2024
Fixes: #9123

The `/` at the beginning of the remote dir path is not necessary when listing files, although it is necessary to download them
The `SftpTemplate.get()` should work also with `remote-dir/MyFile.csv` as input.

* Fix `SftpSession.readRaw()` to call `sftpClient.canonicalPath(source)` if the path does not start with a `/`.
Something similar what is does
* Delegate to `SftpSession.readRaw()` from the `SftpSession.read()`
* Reuse `normalizePath()` for `doList()`

(cherry picked from commit a2215af)
@artembilan
Copy link
Member

The canonical path has been fixed.
I need more info about:

two / are present before the remote repository name

Thanks

@SimoneGiusso
Copy link
Author

SimoneGiusso commented May 8, 2024

The canonical path has been fixed. I need more info about:

two / are present before the remote repository name

Thanks

If you enable the logging level to debug, you should see that file_remoteDirectory header for the message emitted contains the remote_repository path and two /.

Example:

//my_remote_repository instead of just /my_remote_repository

But's this is a minor problem.

@artembilan
Copy link
Member

Sure! But what I have to run to follow such a debug procedure ?

@SimoneGiusso
Copy link
Author

SimoneGiusso commented May 8, 2024

Sure! But what I have to run to follow such a debug procedure ?

If you run my demo in the other opened issue with this config in the application-test.yaml file:

logging:
  level:
    org.springframework.integration.*: debug

You should see logged something similar to

2024-05-08T13:33:19.407+02:00 DEBUG 35630 --- [   scheduling-1] o.s.integration.channel.DirectChannel    : postSend (sent=true) on channel 'bean 'sftpChannel'', message: GenericMessage [payload=/tmp/extractions/MyFile1_data.csv, headers={file_remoteHostPort=localhost:2222, file_name=MyFile1_data.csv, file_remoteDirectory=//deliveries, file_originalFile=/tmp/extractions/MyFile1_data.csv, id=7d1b4bd1-9452-c652-403d-6c607498e5c6, file_relativePath=MyFile1_data.csv, file_remoteFile=MyFile1_data.csv, timestamp=1715167999402}]

As you'll see file_remoteDirectory value has two '/'

@artembilan
Copy link
Member

Thanks, @SimoneGiusso .
Unfortunately, from GitHub it is not obvious that we may talk about the same sample.
So, would be great to point there properly.
Here it is anyway: #9124 (comment)

@SimoneGiusso
Copy link
Author

Thanks, @SimoneGiusso .

Unfortunately, from GitHub it is not obvious that we may talk about the same sample.

So, would be great to point there properly.

Here it is anyway: #9124 (comment)

Yes it is this one. Thanks

@artembilan
Copy link
Member

Right. When I change my test for this .remoteDirectory("/sftpSource"), I indeed se double / in the file_remoteDirectory header.
Will fix shortly: #9129

EddieChoCho pushed a commit to EddieChoCho/spring-integration that referenced this issue Jun 26, 2024
Fixes: spring-projects#9123

The `/` at the beginning of the remote dir path is not necessary when listing files, although it is necessary to download them
The `SftpTemplate.get()` should work also with `remote-dir/MyFile.csv` as input.

* Fix `SftpSession.readRaw()` to call `sftpClient.canonicalPath(source)` if the path does not start with a `/`.
Something similar what is does
* Delegate to `SftpSession.readRaw()` from the `SftpSession.read()`
* Reuse `normalizePath()` for `doList()`

**Auto-cherry-pick to `6.2.x` & `6.1.x`**
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