You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by Kiran Khopade <kh...@adobe.com.INVALID> on 2017/11/09 10:54:25 UTC
ApacheHCHttpCommandExecutorService->convert(HttpRequest request)
regenerating MD5 though it is available in payload metadata
Hi,
I'm generally just a lurker on this list but we use jclouds internally here for various things. I was just trying to understand the implementation of ApacheHCHttpCommandExecutorService. While doing so, I could not understand following code snippet from [drivers>apachehc]ApacheHCHttpCommandExecutorService.java->convert(HttpRequest request) method. (Please refer the following code snippet, highlighted if condition)
protected HttpUriRequest convert(HttpRequest request) throws IOException {
HttpUriRequest returnVal = apacheHCUtils.convertToApacheRequest(request);
if (request.getPayload() != null && request.getPayload().getContentMetadata().getContentMD5() != null) {
String md5 = base64().encode(ByteStreams2.hashAndClose(request.getPayload().openStream(), md5()).asBytes());
returnVal.addHeader("Content-MD5", md5);
}
if (!returnVal.containsHeader(HttpHeaders.USER_AGENT)) {
returnVal.addHeader(HttpHeaders.USER_AGENT, userAgent);
}
return returnVal;
}
I have couple of queries on this if condition code -
1. I Could not understand the purpose of generating MD5 hashcode of payload, if it is already available in payload metadata.
2. While generating md5 of payload in this heighlighted if condition snippet, InputStream is consumed. And as it is consumed to generate MD5, payload will be empty.
Please put some lights on this. We are blocked on this.
Thank You,
Kiran
Re: ApacheHCHttpCommandExecutorService->convert(HttpRequest request)
regenerating MD5 though it is available in payload metadata
Posted by Ignasi Barrera <na...@apache.org>.
Merged! Thanks!
On 10 November 2017 at 09:48, Kiran Khopade <kh...@adobe.com.invalid> wrote:
> Hi Andrew, Ignasi
> I have raised a PR with change to utilize MD5 hash present in the payload contentMetadata, instead of generating it from payload.
>
> Thank You,
> Kiran
>
> -----Original Message-----
> From: Andrew Gaul [mailto:gaul@apache.org]
> Sent: Thursday, November 9, 2017 11:55 PM
> To: dev@jclouds.apache.org
> Cc: Kiran Khopade <kh...@adobe.com>
> Subject: Re: ApacheHCHttpCommandExecutorService->convert(HttpRequest request) regenerating MD5 though it is available in payload metadata
>
> This code was added in e8d0a11cda55328ea25fb1abb3183694f8c771db and I agree that it should not exist. This code does not work for non-repeatable payloads like InputStream. Users should provide MD5 instead of implicitly doing so due to CPU overhead. Kiran could you submit a pull request to fix this?
>
> On Thu, Nov 09, 2017 at 12:07:06PM +0100, Ignasi Barrera wrote:
>> Hi Kiran,
>>
>> I don't know why the MD5 hash is being re-computed, and it looks like
>> it is the only jclouds HTTP driver that does this. In fact, this was
>> discussed some time ago, and it was proven unnecessary:
>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fs.apa
>> che.org%2FCPuh&data=02%7C01%7C%7Cceed09435a0444dde4f608d5279f3788%7Cfa
>> 7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636458487140062213&sdata=iKr6
>> pGMlb8Bw1r8T7BBf4Gnhuye7GG375liALycS7hs%3D&reserved=0
>>
>> I'd say it would be safe to apply the patch mentioned in the linked
>> thread. If you could try it and confirm it fixes your issue, I'd be
>> happy to appy it or to merge a pull request :)
>>
>> @gaul, do you have any input on why could the Apache driver be doing this?
>>
>> On 9 November 2017 at 11:54, Kiran Khopade <kh...@adobe.com.invalid> wrote:
>> > Hi,
>> >
>> > I'm generally just a lurker on this list but we use jclouds
>> > internally here for various things. I was just trying to understand
>> > the implementation of ApacheHCHttpCommandExecutorService. While
>> > doing so, I could not understand following code snippet from
>> > [drivers>apachehc]ApacheHCHttpCommandExecutorService.java->convert(H
>> > ttpRequest request) method. (Please refer the following code
>> > snippet, highlighted if condition)
>> >
>> >
>> > protected HttpUriRequest convert(HttpRequest request) throws IOException {
>> > HttpUriRequest returnVal = apacheHCUtils.convertToApacheRequest(request);
>> > if (request.getPayload() != null && request.getPayload().getContentMetadata().getContentMD5() != null) {
>> > String md5 = base64().encode(ByteStreams2.hashAndClose(request.getPayload().openStream(), md5()).asBytes());
>> > returnVal.addHeader("Content-MD5", md5);
>> > }
>> >
>> > if (!returnVal.containsHeader(HttpHeaders.USER_AGENT)) {
>> > returnVal.addHeader(HttpHeaders.USER_AGENT, userAgent);
>> > }
>> >
>> > return returnVal;
>> > }
>> >
>> > I have couple of queries on this if condition code -
>> >
>> > 1. I Could not understand the purpose of generating MD5 hashcode of payload, if it is already available in payload metadata.
>> > 2. While generating md5 of payload in this heighlighted if condition snippet, InputStream is consumed. And as it is consumed to generate MD5, payload will be empty.
>> >
>> > Please put some lights on this. We are blocked on this.
>> >
>> > Thank You,
>> > Kiran
>> >
>> >
>
> --
> Andrew Gaul
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgaul.org%2F&data=02%7C01%7C%7Cceed09435a0444dde4f608d5279f3788%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636458487140062213&sdata=V%2FEijTYheLgoQ9GVeQoPqnVlihSKgZyYSnDQLBRP%2BHA%3D&reserved=0
RE: ApacheHCHttpCommandExecutorService->convert(HttpRequest request)
regenerating MD5 though it is available in payload metadata
Posted by Kiran Khopade <kh...@adobe.com.INVALID>.
Hi Andrew, Ignasi
I have raised a PR with change to utilize MD5 hash present in the payload contentMetadata, instead of generating it from payload.
Thank You,
Kiran
-----Original Message-----
From: Andrew Gaul [mailto:gaul@apache.org]
Sent: Thursday, November 9, 2017 11:55 PM
To: dev@jclouds.apache.org
Cc: Kiran Khopade <kh...@adobe.com>
Subject: Re: ApacheHCHttpCommandExecutorService->convert(HttpRequest request) regenerating MD5 though it is available in payload metadata
This code was added in e8d0a11cda55328ea25fb1abb3183694f8c771db and I agree that it should not exist. This code does not work for non-repeatable payloads like InputStream. Users should provide MD5 instead of implicitly doing so due to CPU overhead. Kiran could you submit a pull request to fix this?
On Thu, Nov 09, 2017 at 12:07:06PM +0100, Ignasi Barrera wrote:
> Hi Kiran,
>
> I don't know why the MD5 hash is being re-computed, and it looks like
> it is the only jclouds HTTP driver that does this. In fact, this was
> discussed some time ago, and it was proven unnecessary:
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fs.apa
> che.org%2FCPuh&data=02%7C01%7C%7Cceed09435a0444dde4f608d5279f3788%7Cfa
> 7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636458487140062213&sdata=iKr6
> pGMlb8Bw1r8T7BBf4Gnhuye7GG375liALycS7hs%3D&reserved=0
>
> I'd say it would be safe to apply the patch mentioned in the linked
> thread. If you could try it and confirm it fixes your issue, I'd be
> happy to appy it or to merge a pull request :)
>
> @gaul, do you have any input on why could the Apache driver be doing this?
>
> On 9 November 2017 at 11:54, Kiran Khopade <kh...@adobe.com.invalid> wrote:
> > Hi,
> >
> > I'm generally just a lurker on this list but we use jclouds
> > internally here for various things. I was just trying to understand
> > the implementation of ApacheHCHttpCommandExecutorService. While
> > doing so, I could not understand following code snippet from
> > [drivers>apachehc]ApacheHCHttpCommandExecutorService.java->convert(H
> > ttpRequest request) method. (Please refer the following code
> > snippet, highlighted if condition)
> >
> >
> > protected HttpUriRequest convert(HttpRequest request) throws IOException {
> > HttpUriRequest returnVal = apacheHCUtils.convertToApacheRequest(request);
> > if (request.getPayload() != null && request.getPayload().getContentMetadata().getContentMD5() != null) {
> > String md5 = base64().encode(ByteStreams2.hashAndClose(request.getPayload().openStream(), md5()).asBytes());
> > returnVal.addHeader("Content-MD5", md5);
> > }
> >
> > if (!returnVal.containsHeader(HttpHeaders.USER_AGENT)) {
> > returnVal.addHeader(HttpHeaders.USER_AGENT, userAgent);
> > }
> >
> > return returnVal;
> > }
> >
> > I have couple of queries on this if condition code -
> >
> > 1. I Could not understand the purpose of generating MD5 hashcode of payload, if it is already available in payload metadata.
> > 2. While generating md5 of payload in this heighlighted if condition snippet, InputStream is consumed. And as it is consumed to generate MD5, payload will be empty.
> >
> > Please put some lights on this. We are blocked on this.
> >
> > Thank You,
> > Kiran
> >
> >
--
Andrew Gaul
https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgaul.org%2F&data=02%7C01%7C%7Cceed09435a0444dde4f608d5279f3788%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636458487140062213&sdata=V%2FEijTYheLgoQ9GVeQoPqnVlihSKgZyYSnDQLBRP%2BHA%3D&reserved=0
Re: ApacheHCHttpCommandExecutorService->convert(HttpRequest request)
regenerating MD5 though it is available in payload metadata
Posted by Andrew Gaul <ga...@apache.org>.
This code was added in e8d0a11cda55328ea25fb1abb3183694f8c771db and I
agree that it should not exist. This code does not work for
non-repeatable payloads like InputStream. Users should provide MD5
instead of implicitly doing so due to CPU overhead. Kiran could you
submit a pull request to fix this?
On Thu, Nov 09, 2017 at 12:07:06PM +0100, Ignasi Barrera wrote:
> Hi Kiran,
>
> I don't know why the MD5 hash is being re-computed, and it looks like
> it is the only jclouds HTTP driver that does this. In fact, this was
> discussed some time ago, and it was proven unnecessary:
> https://s.apache.org/CPuh
>
> I'd say it would be safe to apply the patch mentioned in the linked
> thread. If you could try it and confirm it fixes your issue, I'd be
> happy to appy it or to merge a pull request :)
>
> @gaul, do you have any input on why could the Apache driver be doing this?
>
> On 9 November 2017 at 11:54, Kiran Khopade <kh...@adobe.com.invalid> wrote:
> > Hi,
> >
> > I'm generally just a lurker on this list but we use jclouds internally here for various things. I was just trying to understand the implementation of ApacheHCHttpCommandExecutorService. While doing so, I could not understand following code snippet from [drivers>apachehc]ApacheHCHttpCommandExecutorService.java->convert(HttpRequest request) method. (Please refer the following code snippet, highlighted if condition)
> >
> >
> > protected HttpUriRequest convert(HttpRequest request) throws IOException {
> > HttpUriRequest returnVal = apacheHCUtils.convertToApacheRequest(request);
> > if (request.getPayload() != null && request.getPayload().getContentMetadata().getContentMD5() != null) {
> > String md5 = base64().encode(ByteStreams2.hashAndClose(request.getPayload().openStream(), md5()).asBytes());
> > returnVal.addHeader("Content-MD5", md5);
> > }
> >
> > if (!returnVal.containsHeader(HttpHeaders.USER_AGENT)) {
> > returnVal.addHeader(HttpHeaders.USER_AGENT, userAgent);
> > }
> >
> > return returnVal;
> > }
> >
> > I have couple of queries on this if condition code -
> >
> > 1. I Could not understand the purpose of generating MD5 hashcode of payload, if it is already available in payload metadata.
> > 2. While generating md5 of payload in this heighlighted if condition snippet, InputStream is consumed. And as it is consumed to generate MD5, payload will be empty.
> >
> > Please put some lights on this. We are blocked on this.
> >
> > Thank You,
> > Kiran
> >
> >
--
Andrew Gaul
http://gaul.org/
Re: ApacheHCHttpCommandExecutorService->convert(HttpRequest request)
regenerating MD5 though it is available in payload metadata
Posted by Ignasi Barrera <na...@apache.org>.
Hi Kiran,
I don't know why the MD5 hash is being re-computed, and it looks like
it is the only jclouds HTTP driver that does this. In fact, this was
discussed some time ago, and it was proven unnecessary:
https://s.apache.org/CPuh
I'd say it would be safe to apply the patch mentioned in the linked
thread. If you could try it and confirm it fixes your issue, I'd be
happy to appy it or to merge a pull request :)
@gaul, do you have any input on why could the Apache driver be doing this?
On 9 November 2017 at 11:54, Kiran Khopade <kh...@adobe.com.invalid> wrote:
> Hi,
>
> I'm generally just a lurker on this list but we use jclouds internally here for various things. I was just trying to understand the implementation of ApacheHCHttpCommandExecutorService. While doing so, I could not understand following code snippet from [drivers>apachehc]ApacheHCHttpCommandExecutorService.java->convert(HttpRequest request) method. (Please refer the following code snippet, highlighted if condition)
>
>
> protected HttpUriRequest convert(HttpRequest request) throws IOException {
> HttpUriRequest returnVal = apacheHCUtils.convertToApacheRequest(request);
> if (request.getPayload() != null && request.getPayload().getContentMetadata().getContentMD5() != null) {
> String md5 = base64().encode(ByteStreams2.hashAndClose(request.getPayload().openStream(), md5()).asBytes());
> returnVal.addHeader("Content-MD5", md5);
> }
>
> if (!returnVal.containsHeader(HttpHeaders.USER_AGENT)) {
> returnVal.addHeader(HttpHeaders.USER_AGENT, userAgent);
> }
>
> return returnVal;
> }
>
> I have couple of queries on this if condition code -
>
> 1. I Could not understand the purpose of generating MD5 hashcode of payload, if it is already available in payload metadata.
> 2. While generating md5 of payload in this heighlighted if condition snippet, InputStream is consumed. And as it is consumed to generate MD5, payload will be empty.
>
> Please put some lights on this. We are blocked on this.
>
> Thank You,
> Kiran
>
>