RE: CB-10974 FileTransfer chunkedMode=false + HTTPS
Hi guys, Shazron - in fact we have the checks in the onprogress events in the doc checking if an event is computable (and it was false for chunkedMode=true), so docs are valid. Could you please review and comment on these PRs? 1. https://github.com/apache/cordova-plugin-file-transfer/pull/169 - this does not force chunkedMode=true for HTTPS if it was specified as false in the options, 2. https://github.com/apache/cordova-plugin-file-transfer/pull/170 - this adds back Content-Length to chunkedMode=true case on iOS so that progress events will be computable and contain total. Please let me know if you have any questions or considerations. Best regards, Sergey Shakhnazarov, Akvelon developer. -Original Message- From: Shazron [mailto:shaz...@gmail.com] Sent: Thursday, November 24, 2016 03:06 To: dev@cordova.apache.org Subject: Re: CB-10974 FileTransfer chunkedMode=false + HTTPS Microsoft emails are still being sent to spam for me (using Gmail) so I didn't see this until later. You said: "should we care about specification or about functionality?" We care firstly about what we documented. In this case, according to our docs, chunkedMode defaults to true, and in our example we show the use of an onprogress event. This runs counter to what you have observed -- there are no progress events. In my opinion, at first glance, this is a bug in what we are documenting should happen, therefore we should have progress events for chunkedMode=true (the default) -- and we record the quirk which I recommend a major version bump for. On Wed, Nov 23, 2016 at 7:14 AM, Sergey Shakhnazarov (Akvelon) < v-ses...@microsoft.com> wrote: > There is a related issue on Ios platform [3] - having chunkedMode=true > causes progress not to be computable as we don't include > Content-Length header to the request in this case. > Reverting that logic fixes the onprogress event but breaks our tests - > so this is a question again - should we care about specification or > about functionality? > Should we revert this to make the progress computable for chunkedMode=true? > > Does anyone has knowledge to advise about the drawbacks of this > specific decision would it be taken (tests show that progress is being > reported correctly but this is again is kind of an internal > implementation we will depend on)? > > [3]: > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fissue > s.apache.org%2Fjira%2Fbrowse%2FCB-12154=02%7C01%7Cv-seshak%40micr > osoft.com%7Cc12922d6191348652e0208d413fdd724%7C72f988bf86f141af91ab2d7 > cd011db47%7C1%7C0%7C636155428348550982=Mdp5yQ6CcT5igUeRukjhf8vb% > 2Bm6bgBMb1sV6lrOfmAE%3D=0 > > Please let me know if you have any questions or considerations. > > Best regards, > Sergey Shakhnazarov. > > -Original Message- > From: Sergey Shakhnazarov (Akvelon) [mailto:v-ses...@microsoft.com] > Sent: Wednesday, November 23, 2016 14:47 > To: dev@cordova.apache.org > Subject: CB-10974 FileTransfer chunkedMode=false + HTTPS > > Hi guys, > > There were several user reports recently about the upload failures > caused by the lack of Content-Length header in case of HTTPS uploads. > This issue is caused by the FileTransfer code, which forces > chunkedMode=true for HTTPS uploads due to possible OutOfMemoryException. > As a solution I've send a PR [1], which does not touch chunkedMode if > it was specified as false in the UploadOptions. > > Do you think this a correct solution? > > According to the HTTP specification [2]: > > Ø Messages MUST NOT include both a Content-Length header field and a > non-identity transfer-coding. If the message does include a non- > identity transfer-coding, the Content-Length MUST be ignored. > > > > So including a Content-Length for the chunkedMode=true case would be wrong. > > Furthermore we don't control the underlying implementation so can't > define the summary length of chunks beforehand. > > > > [1]: https://na01.safelinks.protection.outlook.com/?url= > https%3A%2F%2Fgithub.com%2Fapache%2Fcordova-plugin- > file-transfer%2Fpull%2F169=02%7C01%7Cv-seshak%40microsoft.com% > 7C9a4c5183e2ef4ac549d008d413968086%7C72f988bf86f141af91ab2d7cd011 > db47%7C1%7C0%7C636154984504057421=ggfYGmC0FX% > 2F2rclFaF8T1PuNAUARywO0LSUxHIRXecE%3D=0 > > [2]: https://na01.safelinks.protection.outlook.com/?url= > https%3A%2F%2Fwww.w3.org%2FProtocols%2Frfc2616% > 2Frfc2616-sec4.html%23sec4.4=02%7C01%7Cv-seshak%40microsoft.com% > 7C9a4c5183e2ef4ac549d008d413968086%7C72f988bf86f141af91ab2d7cd011 > db47%7C1%7C0%7C636154984504057421=hDLgzBvz% > 2FvKkq50Oo2R04Q9kmP6rZAQbpXpiD9hgHEI%3D=0 > > Please let me know if you have any questions or considerations. > > Best regards, > Sergey Shakhnazarov. > > > > - > To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org > For additional commands, e-mail: dev-h...@cordova.apache.org > >
Re: CB-10974 FileTransfer chunkedMode=false + HTTPS
Microsoft emails are still being sent to spam for me (using Gmail) so I didn't see this until later. You said: "should we care about specification or about functionality?" We care firstly about what we documented. In this case, according to our docs, chunkedMode defaults to true, and in our example we show the use of an onprogress event. This runs counter to what you have observed -- there are no progress events. In my opinion, at first glance, this is a bug in what we are documenting should happen, therefore we should have progress events for chunkedMode=true (the default) -- and we record the quirk which I recommend a major version bump for. On Wed, Nov 23, 2016 at 7:14 AM, Sergey Shakhnazarov (Akvelon) < v-ses...@microsoft.com> wrote: > There is a related issue on Ios platform [3] - having chunkedMode=true > causes progress not to be computable as we don't include Content-Length > header to the request in this case. > Reverting that logic fixes the onprogress event but breaks our tests - so > this is a question again - should we care about specification or about > functionality? > Should we revert this to make the progress computable for chunkedMode=true? > > Does anyone has knowledge to advise about the drawbacks of this specific > decision would it be taken (tests show that progress is being reported > correctly but this is again is kind of an internal implementation we will > depend on)? > > [3]: https://issues.apache.org/jira/browse/CB-12154 > > Please let me know if you have any questions or considerations. > > Best regards, > Sergey Shakhnazarov. > > -Original Message- > From: Sergey Shakhnazarov (Akvelon) [mailto:v-ses...@microsoft.com] > Sent: Wednesday, November 23, 2016 14:47 > To: dev@cordova.apache.org > Subject: CB-10974 FileTransfer chunkedMode=false + HTTPS > > Hi guys, > > There were several user reports recently about the upload failures caused > by the lack of Content-Length header in case of HTTPS uploads. > This issue is caused by the FileTransfer code, which forces > chunkedMode=true for HTTPS uploads due to possible OutOfMemoryException. > As a solution I've send a PR [1], which does not touch chunkedMode if it > was specified as false in the UploadOptions. > > Do you think this a correct solution? > > According to the HTTP specification [2]: > > Ø Messages MUST NOT include both a Content-Length header field and a > non-identity transfer-coding. If the message does include a non- identity > transfer-coding, the Content-Length MUST be ignored. > > > > So including a Content-Length for the chunkedMode=true case would be wrong. > > Furthermore we don't control the underlying implementation so can't define > the summary length of chunks beforehand. > > > > [1]: https://na01.safelinks.protection.outlook.com/?url= > https%3A%2F%2Fgithub.com%2Fapache%2Fcordova-plugin- > file-transfer%2Fpull%2F169=02%7C01%7Cv-seshak%40microsoft.com% > 7C9a4c5183e2ef4ac549d008d413968086%7C72f988bf86f141af91ab2d7cd011 > db47%7C1%7C0%7C636154984504057421=ggfYGmC0FX% > 2F2rclFaF8T1PuNAUARywO0LSUxHIRXecE%3D=0 > > [2]: https://na01.safelinks.protection.outlook.com/?url= > https%3A%2F%2Fwww.w3.org%2FProtocols%2Frfc2616% > 2Frfc2616-sec4.html%23sec4.4=02%7C01%7Cv-seshak%40microsoft.com% > 7C9a4c5183e2ef4ac549d008d413968086%7C72f988bf86f141af91ab2d7cd011 > db47%7C1%7C0%7C636154984504057421=hDLgzBvz% > 2FvKkq50Oo2R04Q9kmP6rZAQbpXpiD9hgHEI%3D=0 > > Please let me know if you have any questions or considerations. > > Best regards, > Sergey Shakhnazarov. > > > > - > To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org > For additional commands, e-mail: dev-h...@cordova.apache.org > >
RE: CB-10974 FileTransfer chunkedMode=false + HTTPS
There is a related issue on Ios platform [3] - having chunkedMode=true causes progress not to be computable as we don't include Content-Length header to the request in this case. Reverting that logic fixes the onprogress event but breaks our tests - so this is a question again - should we care about specification or about functionality? Should we revert this to make the progress computable for chunkedMode=true? Does anyone has knowledge to advise about the drawbacks of this specific decision would it be taken (tests show that progress is being reported correctly but this is again is kind of an internal implementation we will depend on)? [3]: https://issues.apache.org/jira/browse/CB-12154 Please let me know if you have any questions or considerations. Best regards, Sergey Shakhnazarov. -Original Message- From: Sergey Shakhnazarov (Akvelon) [mailto:v-ses...@microsoft.com] Sent: Wednesday, November 23, 2016 14:47 To: dev@cordova.apache.org Subject: CB-10974 FileTransfer chunkedMode=false + HTTPS Hi guys, There were several user reports recently about the upload failures caused by the lack of Content-Length header in case of HTTPS uploads. This issue is caused by the FileTransfer code, which forces chunkedMode=true for HTTPS uploads due to possible OutOfMemoryException. As a solution I've send a PR [1], which does not touch chunkedMode if it was specified as false in the UploadOptions. Do you think this a correct solution? According to the HTTP specification [2]: Ø Messages MUST NOT include both a Content-Length header field and a non-identity transfer-coding. If the message does include a non- identity transfer-coding, the Content-Length MUST be ignored. So including a Content-Length for the chunkedMode=true case would be wrong. Furthermore we don't control the underlying implementation so can't define the summary length of chunks beforehand. [1]: https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fcordova-plugin-file-transfer%2Fpull%2F169=02%7C01%7Cv-seshak%40microsoft.com%7C9a4c5183e2ef4ac549d008d413968086%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636154984504057421=ggfYGmC0FX%2F2rclFaF8T1PuNAUARywO0LSUxHIRXecE%3D=0 [2]: https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.w3.org%2FProtocols%2Frfc2616%2Frfc2616-sec4.html%23sec4.4=02%7C01%7Cv-seshak%40microsoft.com%7C9a4c5183e2ef4ac549d008d413968086%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636154984504057421=hDLgzBvz%2FvKkq50Oo2R04Q9kmP6rZAQbpXpiD9hgHEI%3D=0 Please let me know if you have any questions or considerations. Best regards, Sergey Shakhnazarov. - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
