You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2022/06/06 02:27:56 UTC
[GitHub] [camel] zhfeng opened a new pull request, #7727: CAMEL-18167: camel-aws2-s3 - Show a message when download link url is…
zhfeng opened a new pull request, #7727:
URL: https://github.com/apache/camel/pull/7727
… not browser compatible
<!-- Uncomment and fill this section if your PR is not trivial
- [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes.
- [ ] Each commit in the pull request should have a meaningful subject line and body.
- [ ] If you're unsure, you can format the pull request title like `[CAMEL-XXX] Fixes bug in camel-file component`, where you replace `CAMEL-XXX` with the appropriate JIRA issue.
- [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [ ] Run `mvn clean install -Psourcecheck` in your module with source check enabled to make sure basic checks pass and there are no checkstyle violations. A more thorough check will be performed on your pull request automatically.
Below are the contribution guidelines:
https://github.com/apache/camel/blob/main/CONTRIBUTING.md
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] davsclaus commented on a diff in pull request #7727: CAMEL-18167: camel-aws2-s3 - Show a message when download link url is…
Posted by GitBox <gi...@apache.org>.
davsclaus commented on code in PR #7727:
URL: https://github.com/apache/camel/pull/7727#discussion_r890989750
##########
components/camel-aws/camel-aws2-s3/src/main/java/org/apache/camel/component/aws2/s3/AWS2S3Producer.java:
##########
@@ -579,6 +579,10 @@ private void createDownloadLink(Exchange exchange) {
.build();
PresignedGetObjectRequest presignedGetObjectRequest = presigner.presignGetObject(getObjectPresignRequest);
+ if (!presignedGetObjectRequest.isBrowserExecutable()) {
Review Comment:
and if its not browser compatible then the url cannot really be used to download from a brwoser, and you need all this workaround code you pointed to?
And because the presigner is closed in camel, then I would not think you can return a header with an input stream as the stream would then be closed
So maybe we do
1) return nothing as a download url is not possible
2) add option to allow to eager download if not browser compabile, but this will then return a blob instead of an url and the returned data can be very big if the s3 file is very big (this can be confusing as either you get a url or a big payload)
3) something else ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] github-actions[bot] commented on pull request #7727: CAMEL-18167: camel-aws2-s3 - Show a message when download link url is…
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #7727:
URL: https://github.com/apache/camel/pull/7727#issuecomment-1147583543
:heavy_check_mark: Finished component verification: 0 component(s) test failed out of **1 component(s) tested**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] github-actions[bot] commented on pull request #7727: CAMEL-18167: camel-aws2-s3 - Show a message when download link url is…
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #7727:
URL: https://github.com/apache/camel/pull/7727#issuecomment-1149637741
:heavy_check_mark: Finished component verification: 0 component(s) test failed out of **1 component(s) tested**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] github-actions[bot] commented on pull request #7727: CAMEL-18167: camel-aws2-s3 - Show a message when download link url is…
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #7727:
URL: https://github.com/apache/camel/pull/7727#issuecomment-1147636221
:heavy_check_mark: Finished component verification: 0 component(s) test failed out of **1 component(s) tested**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] davsclaus commented on a diff in pull request #7727: CAMEL-18167: camel-aws2-s3 - Show a message when download link url is…
Posted by GitBox <gi...@apache.org>.
davsclaus commented on code in PR #7727:
URL: https://github.com/apache/camel/pull/7727#discussion_r889895889
##########
components/camel-aws/camel-aws2-s3/src/main/java/org/apache/camel/component/aws2/s3/AWS2S3Producer.java:
##########
@@ -579,6 +579,10 @@ private void createDownloadLink(Exchange exchange) {
.build();
PresignedGetObjectRequest presignedGetObjectRequest = presigner.presignGetObject(getObjectPresignRequest);
+ if (!presignedGetObjectRequest.isBrowserExecutable()) {
Review Comment:
So if every message is like this you have a INFO logging per message, that can spam the logs if you have a lot of files to upload.
Either reduce the logging level to DEBUG or we need a way to only log INFO once and then tell that this can happen for other files too.
Maybe there can be a header added in the response that has this status? Then users can use this in their camel routes as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] zhfeng merged pull request #7727: CAMEL-18167: camel-aws2-s3 - Show a message when download link url is…
Posted by GitBox <gi...@apache.org>.
zhfeng merged PR #7727:
URL: https://github.com/apache/camel/pull/7727
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] zhfeng commented on a diff in pull request #7727: CAMEL-18167: camel-aws2-s3 - Show a message when download link url is…
Posted by GitBox <gi...@apache.org>.
zhfeng commented on code in PR #7727:
URL: https://github.com/apache/camel/pull/7727#discussion_r889915357
##########
components/camel-aws/camel-aws2-s3/src/main/java/org/apache/camel/component/aws2/s3/AWS2S3Producer.java:
##########
@@ -579,6 +579,10 @@ private void createDownloadLink(Exchange exchange) {
.build();
PresignedGetObjectRequest presignedGetObjectRequest = presigner.presignGetObject(getObjectPresignRequest);
+ if (!presignedGetObjectRequest.isBrowserExecutable()) {
Review Comment:
yeah, I get it. so if `s3browsercompatible` is true, user have to send some signed payload. I just wonder if we can pass it through the router?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] github-actions[bot] commented on pull request #7727: CAMEL-18167: camel-aws2-s3 - Show a message when download link url is…
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #7727:
URL: https://github.com/apache/camel/pull/7727#issuecomment-1146971140
:warning: This PR changes Camel components and will be tested automatically.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] zhfeng commented on a diff in pull request #7727: CAMEL-18167: camel-aws2-s3 - Show a message when download link url is…
Posted by GitBox <gi...@apache.org>.
zhfeng commented on code in PR #7727:
URL: https://github.com/apache/camel/pull/7727#discussion_r890214426
##########
components/camel-aws/camel-aws2-s3/src/main/java/org/apache/camel/component/aws2/s3/AWS2S3Producer.java:
##########
@@ -579,6 +579,10 @@ private void createDownloadLink(Exchange exchange) {
.build();
PresignedGetObjectRequest presignedGetObjectRequest = presigner.presignGetObject(getObjectPresignRequest);
+ if (!presignedGetObjectRequest.isBrowserExecutable()) {
Review Comment:
Hi @davsclaus, I add a header `DOWNLOAD_LINK_BROWSER_COMPATIBLE` to store the staus. But I'm not very sure that it is possible to add another header to store a signed payload which type is `InputStream`. Is it safe to copy it when adding in message headers?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] zhfeng commented on a diff in pull request #7727: CAMEL-18167: camel-aws2-s3 - Show a message when download link url is…
Posted by GitBox <gi...@apache.org>.
zhfeng commented on code in PR #7727:
URL: https://github.com/apache/camel/pull/7727#discussion_r889906479
##########
components/camel-aws/camel-aws2-s3/src/main/java/org/apache/camel/component/aws2/s3/AWS2S3Producer.java:
##########
@@ -579,6 +579,10 @@ private void createDownloadLink(Exchange exchange) {
.build();
PresignedGetObjectRequest presignedGetObjectRequest = presigner.presignGetObject(getObjectPresignRequest);
+ if (!presignedGetObjectRequest.isBrowserExecutable()) {
Review Comment:
good point - `DEBUG` level makes sense and if `checksum` validation enabled, users have to send a signedPayload. such codes refers to https://sdk.amazonaws.com/java/api/latest/software/amazon/awssdk/services/s3/presigner/S3Presigner.html
```
// Send any request payload that is needed by the service (not needed when isBrowserExecutable is true)
if (presignedRequest.signedPayload().isPresent()) {
connection.setDoOutput(true);
try (InputStream signedPayload = presignedRequest.signedPayload().get().asInputStream();
OutputStream httpOutputStream = connection.getOutputStream()) {
IoUtils.copy(signedPayload, httpOutputStream);
}
}
```
Is it possible to use a header to hold a `InputStream` value?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] zhfeng commented on a diff in pull request #7727: CAMEL-18167: camel-aws2-s3 - Show a message when download link url is…
Posted by GitBox <gi...@apache.org>.
zhfeng commented on code in PR #7727:
URL: https://github.com/apache/camel/pull/7727#discussion_r891146198
##########
components/camel-aws/camel-aws2-s3/src/main/java/org/apache/camel/component/aws2/s3/AWS2S3Producer.java:
##########
@@ -579,6 +579,10 @@ private void createDownloadLink(Exchange exchange) {
.build();
PresignedGetObjectRequest presignedGetObjectRequest = presigner.presignGetObject(getObjectPresignRequest);
+ if (!presignedGetObjectRequest.isBrowserExecutable()) {
Review Comment:
I'd like to add two headers
- `DOWNLOAD_LINK_HTTP_REQUEST_HEADERS` with `presignedGetObjectRequest.httpRequest().headers()`
- `DOWNLOAD_LINK_SIGNED_PAYLOAD` with `presignedGetObjectRequest.signedPayload().asUtf8String()` if present
I just find that signed payload could be read as `String` so it should be fine to be added into message headers. And if the users really need to enable `checksum validation` when getting the dowonload link, it is their repsonsibleity to get these two from the message headers and add them into the http request for downloading the file. We can add this information as a `NOTE` in our aws2-s3.adoc
WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] github-actions[bot] commented on pull request #7727: CAMEL-18167: camel-aws2-s3 - Show a message when download link url is…
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #7727:
URL: https://github.com/apache/camel/pull/7727#issuecomment-1147038431
:heavy_check_mark: Finished component verification: 0 component(s) test failed out of **1 component(s) tested**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] zhfeng commented on a diff in pull request #7727: CAMEL-18167: camel-aws2-s3 - Show a message when download link url is…
Posted by GitBox <gi...@apache.org>.
zhfeng commented on code in PR #7727:
URL: https://github.com/apache/camel/pull/7727#discussion_r890219014
##########
components/camel-aws/camel-aws2-s3/src/main/java/org/apache/camel/component/aws2/s3/AWS2S3Producer.java:
##########
@@ -579,6 +579,10 @@ private void createDownloadLink(Exchange exchange) {
.build();
PresignedGetObjectRequest presignedGetObjectRequest = presigner.presignGetObject(getObjectPresignRequest);
+ if (!presignedGetObjectRequest.isBrowserExecutable()) {
Review Comment:
```
presignedGetObjectRequest.signedPayload().ifPresent(payload -> {
message.setHeader(AWS2S3Constants.DOWNLOAD_LINK_SIGNED_PAYLOAD, payload.asInputStream());
});
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] zhfeng commented on a diff in pull request #7727: CAMEL-18167: camel-aws2-s3 - Show a message when download link url is…
Posted by GitBox <gi...@apache.org>.
zhfeng commented on code in PR #7727:
URL: https://github.com/apache/camel/pull/7727#discussion_r889933086
##########
components/camel-aws/camel-aws2-s3/src/main/java/org/apache/camel/component/aws2/s3/AWS2S3Producer.java:
##########
@@ -579,6 +579,10 @@ private void createDownloadLink(Exchange exchange) {
.build();
PresignedGetObjectRequest presignedGetObjectRequest = presigner.presignGetObject(getObjectPresignRequest);
+ if (!presignedGetObjectRequest.isBrowserExecutable()) {
Review Comment:
use a header to store `InputStream` value for signed payload? :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] github-actions[bot] commented on pull request #7727: CAMEL-18167: camel-aws2-s3 - Show a message when download link url is…
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #7727:
URL: https://github.com/apache/camel/pull/7727#issuecomment-1149583826
:heavy_check_mark: Finished component verification: 0 component(s) test failed out of **1 component(s) tested**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] github-actions[bot] commented on pull request #7727: CAMEL-18167: camel-aws2-s3 - Show a message when download link url is…
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #7727:
URL: https://github.com/apache/camel/pull/7727#issuecomment-1146991677
:x: Finished component verification: **1 component(s) test failed** out of 1 component(s) tested
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] davsclaus commented on a diff in pull request #7727: CAMEL-18167: camel-aws2-s3 - Show a message when download link url is…
Posted by GitBox <gi...@apache.org>.
davsclaus commented on code in PR #7727:
URL: https://github.com/apache/camel/pull/7727#discussion_r889908594
##########
components/camel-aws/camel-aws2-s3/src/main/java/org/apache/camel/component/aws2/s3/AWS2S3Producer.java:
##########
@@ -579,6 +579,10 @@ private void createDownloadLink(Exchange exchange) {
.build();
PresignedGetObjectRequest presignedGetObjectRequest = presigner.presignGetObject(getObjectPresignRequest);
+ if (!presignedGetObjectRequest.isBrowserExecutable()) {
Review Comment:
Oh I was thinking about a header to store a boolean whether it was browser compatible or not
Something ala:
setHeader("s3browsercompatible", true or false)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] davsclaus commented on a diff in pull request #7727: CAMEL-18167: camel-aws2-s3 - Show a message when download link url is…
Posted by GitBox <gi...@apache.org>.
davsclaus commented on code in PR #7727:
URL: https://github.com/apache/camel/pull/7727#discussion_r889921206
##########
components/camel-aws/camel-aws2-s3/src/main/java/org/apache/camel/component/aws2/s3/AWS2S3Producer.java:
##########
@@ -579,6 +579,10 @@ private void createDownloadLink(Exchange exchange) {
.build();
PresignedGetObjectRequest presignedGetObjectRequest = presigner.presignGetObject(getObjectPresignRequest);
+ if (!presignedGetObjectRequest.isBrowserExecutable()) {
Review Comment:
Yes you can use a header for that
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] davsclaus commented on a diff in pull request #7727: CAMEL-18167: camel-aws2-s3 - Show a message when download link url is…
Posted by GitBox <gi...@apache.org>.
davsclaus commented on code in PR #7727:
URL: https://github.com/apache/camel/pull/7727#discussion_r891173703
##########
components/camel-aws/camel-aws2-s3/src/main/java/org/apache/camel/component/aws2/s3/AWS2S3Producer.java:
##########
@@ -579,6 +579,10 @@ private void createDownloadLink(Exchange exchange) {
.build();
PresignedGetObjectRequest presignedGetObjectRequest = presigner.presignGetObject(getObjectPresignRequest);
+ if (!presignedGetObjectRequest.isBrowserExecutable()) {
Review Comment:
Yes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] davsclaus commented on a diff in pull request #7727: CAMEL-18167: camel-aws2-s3 - Show a message when download link url is…
Posted by GitBox <gi...@apache.org>.
davsclaus commented on code in PR #7727:
URL: https://github.com/apache/camel/pull/7727#discussion_r890986307
##########
components/camel-aws/camel-aws2-s3/src/main/java/org/apache/camel/component/aws2/s3/AWS2S3Producer.java:
##########
@@ -579,6 +579,10 @@ private void createDownloadLink(Exchange exchange) {
.build();
PresignedGetObjectRequest presignedGetObjectRequest = presigner.presignGetObject(getObjectPresignRequest);
+ if (!presignedGetObjectRequest.isBrowserExecutable()) {
Review Comment:
Ah so what can / should we do? If the camel component is to get a url for download - so output is a String with the url
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org