slackapi / java-slack-sdk

Slack Developer Kit (including Bolt for Java) for any JVM language
https://slack.dev/java-slack-sdk/
MIT License
571 stars 212 forks source link

Uploading multiple files overrides filename #1345

Closed Cheos137 closed 3 weeks ago

Cheos137 commented 1 month ago

Uploading multiple files using the 'not mainly used for backwards compatibility' overrides filename of all FilesUploadV2Request.UploadFile objects passed to com.slack.api.methods.impl.MethodsClientImpl#filesUploadV2 via com.slack.api.methods.request.files.FilesUploadV2Request#uploadFiles.

Reproducible in:

The Slack SDK version

1.1.6 / model 1.40.3

+--- com.slack.api:bolt:1.1.+ -> 1.1.6
|    +--- com.slack.api:slack-api-model:1.1.6 -> 1.40.3
|    +--- com.slack.api:slack-api-client:1.1.6 -> 1.40.3
|    |    +--- com.slack.api:slack-api-model:1.40.3 (*)
|    \--- com.slack.api:slack-app-backend:1.1.6
|         +--- com.slack.api:slack-api-model:1.1.6 -> 1.40.3 (*)
|         +--- com.slack.api:slack-api-client:1.1.6 -> 1.40.3 (*)
+--- com.slack.api:bolt-servlet:1.1.+ -> 1.1.6
|    +--- com.slack.api:slack-api-model:1.1.6 -> 1.40.3 (*)
|    +--- com.slack.api:slack-api-client:1.1.6 -> 1.40.3 (*)
|    +--- com.slack.api:slack-app-backend:1.1.6 (*)
|    \--- com.slack.api:bolt:1.1.6 (*)
+--- com.slack.api:bolt-jetty:1.1.+ -> 1.1.6
|    +--- com.slack.api:slack-api-model:1.1.6 -> 1.40.3 (*)
|    +--- com.slack.api:slack-api-client:1.1.6 -> 1.40.3 (*)
|    +--- com.slack.api:slack-app-backend:1.1.6 (*)
|    +--- com.slack.api:bolt:1.1.6 (*)
|    +--- com.slack.api:bolt-servlet:1.1.6 (*)
+--- com.slack.api:slack-api-client:1.+ -> 1.40.3 (*)
+--- com.slack.api:bolt:1.1.+ (n)
+--- com.slack.api:bolt-servlet:1.1.+ (n)
+--- com.slack.api:bolt-jetty:1.1.+ (n)
+--- com.slack.api:slack-api-client:1.+ (n)
+--- com.slack.api:bolt:1.1.+ -> 1.1.6
|    +--- com.slack.api:slack-api-model:1.1.6 -> 1.40.3
|    +--- com.slack.api:slack-api-client:1.1.6 -> 1.40.3
|    |    +--- com.slack.api:slack-api-model:1.40.3 (*)
|    \--- com.slack.api:slack-app-backend:1.1.6
|         +--- com.slack.api:slack-api-model:1.1.6 -> 1.40.3 (*)
|         +--- com.slack.api:slack-api-client:1.1.6 -> 1.40.3 (*)
+--- com.slack.api:bolt-servlet:1.1.+ -> 1.1.6
|    +--- com.slack.api:slack-api-model:1.1.6 -> 1.40.3 (*)
|    +--- com.slack.api:slack-api-client:1.1.6 -> 1.40.3 (*)
|    +--- com.slack.api:slack-app-backend:1.1.6 (*)
|    \--- com.slack.api:bolt:1.1.6 (*)
+--- com.slack.api:bolt-jetty:1.1.+ -> 1.1.6
|    +--- com.slack.api:slack-api-model:1.1.6 -> 1.40.3 (*)
|    +--- com.slack.api:slack-api-client:1.1.6 -> 1.40.3 (*)
|    +--- com.slack.api:slack-app-backend:1.1.6 (*)
|    +--- com.slack.api:bolt:1.1.6 (*)
|    +--- com.slack.api:bolt-servlet:1.1.6 (*)
+--- com.slack.api:slack-api-client:1.+ -> 1.40.3 (*)
+--- com.slack.api:bolt:1.1.+ -> 1.1.6
|    +--- com.slack.api:slack-api-model:1.1.6 -> 1.40.3
|    +--- com.slack.api:slack-api-client:1.1.6 -> 1.40.3
|    |    +--- com.slack.api:slack-api-model:1.40.3 (*)
|    \--- com.slack.api:slack-app-backend:1.1.6
|         +--- com.slack.api:slack-api-model:1.1.6 -> 1.40.3 (*)
|         +--- com.slack.api:slack-api-client:1.1.6 -> 1.40.3 (*)
+--- com.slack.api:bolt-servlet:1.1.+ -> 1.1.6
|    +--- com.slack.api:slack-api-model:1.1.6 -> 1.40.3 (*)
|    +--- com.slack.api:slack-api-client:1.1.6 -> 1.40.3 (*)
|    +--- com.slack.api:slack-app-backend:1.1.6 (*)
|    \--- com.slack.api:bolt:1.1.6 (*)
+--- com.slack.api:bolt-jetty:1.1.+ -> 1.1.6
|    +--- com.slack.api:slack-api-model:1.1.6 -> 1.40.3 (*)
|    +--- com.slack.api:slack-api-client:1.1.6 -> 1.40.3 (*)
|    +--- com.slack.api:slack-app-backend:1.1.6 (*)
|    +--- com.slack.api:bolt:1.1.6 (*)
|    +--- com.slack.api:bolt-servlet:1.1.6 (*)
+--- com.slack.api:slack-api-client:1.+ -> 1.40.3 (*)
+--- com.slack.api:bolt:1.1.+ -> 1.1.6
|    +--- com.slack.api:slack-api-model:1.1.6 -> 1.40.3
|    +--- com.slack.api:slack-api-client:1.1.6 -> 1.40.3
|    |    +--- com.slack.api:slack-api-model:1.40.3 (*)
|    \--- com.slack.api:slack-app-backend:1.1.6
|         +--- com.slack.api:slack-api-model:1.1.6 -> 1.40.3 (*)
|         +--- com.slack.api:slack-api-client:1.1.6 -> 1.40.3 (*)
+--- com.slack.api:bolt-servlet:1.1.+ -> 1.1.6
|    +--- com.slack.api:slack-api-model:1.1.6 -> 1.40.3 (*)
|    +--- com.slack.api:slack-api-client:1.1.6 -> 1.40.3 (*)
|    +--- com.slack.api:slack-app-backend:1.1.6 (*)
|    \--- com.slack.api:bolt:1.1.6 (*)
+--- com.slack.api:bolt-jetty:1.1.+ -> 1.1.6
|    +--- com.slack.api:slack-api-model:1.1.6 -> 1.40.3 (*)
|    +--- com.slack.api:slack-api-client:1.1.6 -> 1.40.3 (*)
|    +--- com.slack.api:slack-app-backend:1.1.6 (*)
|    +--- com.slack.api:bolt:1.1.6 (*)
|    +--- com.slack.api:bolt-servlet:1.1.6 (*)
+--- com.slack.api:slack-api-client:1.+ -> 1.40.3 (*)

Java Runtime version

(version-independent/this information is irrelevant)

openjdk version "21.0.4" 2024-07-16
OpenJDK Runtime Environment (build 21.0.4+7-Ubuntu-1ubuntu222.04)
OpenJDK 64-Bit Server VM (build 21.0.4+7-Ubuntu-1ubuntu222.04, mixed mode, sharing)

OS info

(version-independent/this information is irrelevant)

#45~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Mon Jul 15 16:40:02 UTC 2

Steps to reproduce:

(Share the commands to run, source code, and project settings (e.g., pom.xml/build.gradle))

  1. attempt uploading any file using the filesUploadV2 method (it doesn't matter if the FilesUploadV2Request#filename field is blank/null or set to any value)
  2. observe the filename of all files being replaced with Uploaded File (if null was passed) or the passed FilesUploadV2Request#filename
  3. you can bypass this issue by uploading one single file at a time, by using the fields commented with (this is mainly for backward compatibility - using uploadFiles instead is recommended)
  4. inspect the source code (com.slack.api.methods.impl.MethodsClientImpl, line 2341) and confirm the error

Expected result:

The filenames don't get overridden

Actual result:

All filenames are replaced with one single value

How to fix:

in com.slack.api.methods.impl.MethodsClientImpl on line 2341, replace the call to req.getFilename() with a call to uploadFile.getFilename() . This seems to be a simple copy-paste error - though i think it might be useful to check the entire method for additional errors i might've overlooked.

Requirements

Please make sure if this topic is specific to this SDK. For general questions/issues about Slack API platform or its server-side, could you submit questions at https://my.slack.com/help/requests/new instead. :bow:

Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you agree to those rules.

seratch commented 1 month ago

Hi @Cheos137, thanks for taking the time to write in. Indeed, this bug existed in past versions (older versions before v1.39.3), but it was resolved by this PR: https://github.com/slackapi/java-slack-sdk/pull/1316 Could you try the latest version out?

Cheos137 commented 1 month ago

as shown by the dependency graph above, i'm using bolt 1.1.6 with api client/model 1.40.3, checking the pull request i can confirm that line 2341 (HEAD) / 2243 (before pr) / 2239 (with pr applied) where this issue lies is not touched at all by #1316. I've opened an accompanying PR #1346 to fix this.

seratch commented 3 weeks ago

@Cheos137 Thank you so much for taking the time to write code to resolve this issue!