Skip to content

When deleteSourceFiles is true, the setPermissions method of FileWritingMessageHandler should be called. #9294

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
coverseed opened this issue Jul 1, 2024 · 3 comments · Fixed by #9300

Comments

@coverseed
Copy link

Expected Behavior

chmod should need to work when the FileWritingMessageHandler's deleteSourceFiles property is true.
Current Behavior

When the deleteSourceFiles property of FileWritingMessageHandler is true, chmod does not work.
Context

My code is as follows:

@Bean
public IntegrationFlow pacToZipFlow(MessageChannel packToZipChannel,  FileSynchronizerProperties fileSynchronizerProperties) {
	return IntegrationFlow
			.from(packToZipChannel)
			.transformWith(transformer -> transformer.transformer(new ZipTransformer()))
			.handle(Files
					.outboundAdapter(new File(fileSynchronizerProperties.outputDirectory()))
					.fileNameGenerator(message -> System.currentTimeMillis() + FileConsts.ZIP_FILE_SUFFIX)
					.deleteSourceFiles(true)
					.chmod(0700)
			)
			.get();
}

When deleteSourceFiles is true, the setPermissions(resultFile) method is not called in the handleFileMessage method of FileWritingMessageHandler.

@coverseed coverseed added status: waiting-for-triage The issue need to be evaluated and its future decided type: enhancement labels Jul 1, 2024
@artembilan
Copy link
Member

The logic is like this:

		if (this.deleteSourceFiles && originalFile != null && !originalFile.delete()) {
			throw new IllegalStateException("Could not delete original file: " + originalFile);
		}

		setPermissions(resultFile);
	}

	/**
	 * Set permissions on newly written files.
	 * @param resultFile the file.
	 * @throws IOException any exception.
	 * @since 5.0
	 */
	protected void setPermissions(File resultFile) throws IOException {

That setPermissions is for a newly created file.
It has nothing to do with original which you are going to delete.

As you see that setPermissions() is not called only in case of original file delete failure.

Do you mean the setPermissions() has to be unconditional?
Or what is your request about, please?

@artembilan artembilan added status: waiting-for-reporter Needs a feedback from the reporter and removed status: waiting-for-triage The issue need to be evaluated and its future decided labels Jul 1, 2024
@coverseed
Copy link
Author

When I call the handleFileMessage() method, and !FileExistsMode.APPEND.equals(this.fileExistsMode) && this.deleteSourceFiles is true, the rename() method is called directly without calling the setPermissions() method. Can you provide support for this?

	private File handleFileMessage(File sourceFile, File tempFile, File resultFile, Message<?> requestMessage)
			throws IOException {

		if (!FileExistsMode.APPEND.equals(this.fileExistsMode) && this.deleteSourceFiles) {
			rename(sourceFile, resultFile);
			return resultFile;
		}
		else {
			BufferedInputStream bis = new BufferedInputStream(new FileInputStream(sourceFile));
			return handleInputStreamMessage(bis, sourceFile, tempFile, resultFile, requestMessage);
		}
	}

@artembilan
Copy link
Member

Yeah... That use-case is missed.
Good catch!

I can fix it quickly, but wonder if you are willing to contribute: https://github.com/spring-projects/spring-integration/blob/main/CONTRIBUTING.adoc.

Looks like that setPermissions() is supposed to be called after rename().

@artembilan artembilan added this to the 6.4.0-M1 milestone Jul 1, 2024
artembilan added a commit to artembilan/spring-integration that referenced this issue Jul 3, 2024
Fixes: spring-projects#9294

When the `deleteSourceFiles` property of `FileWritingMessageHandler` is `true`, the `chmod` is not set on the target file.

* Fix `FileWritingMessageHandler.handleFileMessage()` logic to `setPermissions(resultFile)` after `move` operation

**Auto-cherry-pick to `6.3.x` & `6.2.x`**
spring-builds pushed a commit that referenced this issue Jul 8, 2024
* GH-9294: Set permissions to target file on rename

Fixes: #9294

When the `deleteSourceFiles` property of `FileWritingMessageHandler` is `true`, the `chmod` is not set on the target file.

* Fix `FileWritingMessageHandler.handleFileMessage()` logic to `setPermissions(resultFile)` after `move` operation

* * Set only `OWNER_READ` permission for file in unit test

(cherry picked from commit 3e71ea5)
spring-builds pushed a commit that referenced this issue Jul 8, 2024
* GH-9294: Set permissions to target file on rename

Fixes: #9294

When the `deleteSourceFiles` property of `FileWritingMessageHandler` is `true`, the `chmod` is not set on the target file.

* Fix `FileWritingMessageHandler.handleFileMessage()` logic to `setPermissions(resultFile)` after `move` operation

* * Set only `OWNER_READ` permission for file in unit test

(cherry picked from commit 3e71ea5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants