Bug #95683 for Artifactory-Client: Uses File::Slurp, known to be buggy and vulnerable

Thu May 15 16:06:56 2014 ether [...] cpan.org - Ticket created

Subject:

Uses File::Slurp, known to be buggy and vulnerable

e.g. look at https://rt.cpan.org/Ticket/Display.html?id=83126 and be dismayed File::Slurp::Tiny and Path::Tiny are both excellent alternatives.

Fri May 23 02:54:38 2014 SYAGI [...] cpan.org - Correspondence added

On Thu May 15 16:06:56 2014, ETHER wrote: Show quoted text

> e.g. look at https://rt.cpan.org/Ticket/Display.html?id=83126 and be > dismayed > > File::Slurp::Tiny and Path::Tiny are both excellent alternatives.

Thank you. I'll look into them and use alternatives as you suggested.

Fri May 23 02:54:38 2014 The RT System itself - Status changed from 'new' to 'open'

Sat May 24 00:54:51 2014 SYAGI [...] cpan.org - Correspondence added

Will look at this next Tuesday after the long weekend. On Fri May 23 02:54:38 2014, SYAGI wrote: Show quoted text

> On Thu May 15 16:06:56 2014, ETHER wrote:

> > e.g. look at https://rt.cpan.org/Ticket/Display.html?id=83126 and be > > dismayed > > > > File::Slurp::Tiny and Path::Tiny are both excellent alternatives.

> > Thank you. I'll look into them and use alternatives as you suggested.

Tue May 27 23:21:54 2014 SYAGI [...] cpan.org - Correspondence added

I looked at the alternatives you've suggested and decided to keep it as-is. Here are my reasons: 1. The issue seems to occur with utf8 mode; I am using File::Slurp for raw mode only. 2. Neither alternative seems as readily available as File::Slurp. For instance, at my workplace we have internal repository and neither module was found in the repo. If you still believe this is an issue in the context of how I am using File::Slurp, please let me know. I can easily remove the File::Slurp usage altogether and open the file using open(). On Sat May 24 00:54:51 2014, SYAGI wrote: Show quoted text

> Will look at this next Tuesday after the long weekend. > > On Fri May 23 02:54:38 2014, SYAGI wrote:

> > On Thu May 15 16:06:56 2014, ETHER wrote:

> > > e.g. look at https://rt.cpan.org/Ticket/Display.html?id=83126 and be > > > dismayed > > > > > > File::Slurp::Tiny and Path::Tiny are both excellent alternatives.

> > > > Thank you. I'll look into them and use alternatives as you suggested.

Tue May 27 23:21:55 2014 SYAGI [...] cpan.org - Status changed from 'open' to 'resolved'

Tue May 27 23:21:55 2014 SYAGI [...] cpan.org - Taken

Fri Jun 20 19:11:24 2014 SYAGI [...] cpan.org - Correspondence added

Looks like File::Slurp actually does impact my module when large files are uploaded. I will reopen this and look for alternatives. On Tue May 27 23:21:54 2014, SYAGI wrote: Show quoted text

> I looked at the alternatives you've suggested and decided to keep it > as-is. Here are my reasons: > > 1. The issue seems to occur with utf8 mode; I am using File::Slurp for > raw mode only. > > 2. Neither alternative seems as readily available as File::Slurp. For > instance, at my workplace we have internal repository and neither > module was found in the repo. > > If you still believe this is an issue in the context of how I am using > File::Slurp, please let me know. I can easily remove the File::Slurp > usage altogether and open the file using open(). > > On Sat May 24 00:54:51 2014, SYAGI wrote:

> > Will look at this next Tuesday after the long weekend. > > > > On Fri May 23 02:54:38 2014, SYAGI wrote:

> > > On Thu May 15 16:06:56 2014, ETHER wrote:

> > > > e.g. look at https://rt.cpan.org/Ticket/Display.html?id=83126 and > > > > be > > > > dismayed > > > > > > > > File::Slurp::Tiny and Path::Tiny are both excellent alternatives.

> > > > > > Thank you. I'll look into them and use alternatives as you > > > suggested.

> >

Fri Jun 20 19:11:24 2014 SYAGI [...] cpan.org - Status changed from 'resolved' to 'open'

Fri Jun 20 20:27:48 2014 ether [...] cpan.org - Correspondence added

Subject:	Re: [rt.cpan.org #95683] Uses File::Slurp, known to be buggy and vulnerable
Date:	Fri, 20 Jun 2014 17:27:35 -0700
To:	Satoshi Yagi via RT <bug-Artifactory-Client [...] rt.cpan.org>
From:	Karen Etheridge <ether [...] cpan.org>

On Fri, Jun 20, 2014 at 07:11:24PM -0400, Satoshi Yagi via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=95683 > > > Looks like File::Slurp actually does impact my module when large files are uploaded. I will reopen this and look for alternatives.

Cool! BTW, this thread might also be of some use, as it explores some of the pitfalls (and solutions). But in general, I highly recommend Path::Tiny. Show quoted text

> On Tue May 27 23:21:54 2014, SYAGI wrote:

> > I looked at the alternatives you've suggested and decided to keep it > > as-is. Here are my reasons: > > > > 1. The issue seems to occur with utf8 mode; I am using File::Slurp for > > raw mode only. > > > > 2. Neither alternative seems as readily available as File::Slurp. For > > instance, at my workplace we have internal repository and neither > > module was found in the repo. > > > > If you still believe this is an issue in the context of how I am using > > File::Slurp, please let me know. I can easily remove the File::Slurp > > usage altogether and open the file using open(). > > > > On Sat May 24 00:54:51 2014, SYAGI wrote:

> > > Will look at this next Tuesday after the long weekend. > > > > > > On Fri May 23 02:54:38 2014, SYAGI wrote:

> > > > On Thu May 15 16:06:56 2014, ETHER wrote:

> > > > > e.g. look at https://rt.cpan.org/Ticket/Display.html?id=83126 and > > > > > be > > > > > dismayed > > > > > > > > > > File::Slurp::Tiny and Path::Tiny are both excellent alternatives.

> > > > > > > > Thank you. I'll look into them and use alternatives as you > > > > suggested.

> > >

> >

-- "I felt a kind of forlorn sense of being lost in a world of incredibly stupid and malicious dwarfs." - Aleister Crowley . . . . . Karen Etheridge, karen@etheridge.ca GCS C+++$ USL+++$ P+++$ w--- M++

Mon Jun 23 23:34:54 2014 SYAGI [...] cpan.org - Correspondence added

I followed your recommendation and used Path::Tiny in version 0.7.0. The bad news is that the "Out of memory" error persists. Basically what's happening is that when you upload a large archive, the module reads the entire file in memory before issuing an HTTP call. I played with LWP / HTTP::Request libraries to see if there's a way to stream a file before making a request; I haven't found one yet. It is really odd because my test instance has 2G memory and it is crapping out on far less memory footprint (about 500MB). In any case, this issue is a Perl issue rather than the module I'm using. I'm open for suggestions / solutions, meanwhile I will wrap up other functionalities. On Fri Jun 20 20:27:48 2014, ETHER wrote: Show quoted text

> On Fri, Jun 20, 2014 at 07:11:24PM -0400, Satoshi Yagi via RT wrote:

> > <URL: https://rt.cpan.org/Ticket/Display.html?id=95683 > > > > > Looks like File::Slurp actually does impact my module when large > > files are uploaded. I will reopen this and look for alternatives.

> > Cool! > > BTW, this thread might also be of some use, as it explores some of the > pitfalls (and solutions). But in general, I highly recommend > Path::Tiny. >

> > On Tue May 27 23:21:54 2014, SYAGI wrote:

> > > I looked at the alternatives you've suggested and decided to keep > > > it > > > as-is. Here are my reasons: > > > > > > 1. The issue seems to occur with utf8 mode; I am using File::Slurp > > > for > > > raw mode only. > > > > > > 2. Neither alternative seems as readily available as File::Slurp. > > > For > > > instance, at my workplace we have internal repository and neither > > > module was found in the repo. > > > > > > If you still believe this is an issue in the context of how I am > > > using > > > File::Slurp, please let me know. I can easily remove the > > > File::Slurp > > > usage altogether and open the file using open(). > > > > > > On Sat May 24 00:54:51 2014, SYAGI wrote:

> > > > Will look at this next Tuesday after the long weekend. > > > > > > > > On Fri May 23 02:54:38 2014, SYAGI wrote:

> > > > > On Thu May 15 16:06:56 2014, ETHER wrote:

> > > > > > e.g. look at https://rt.cpan.org/Ticket/Display.html?id=83126 > > > > > > and > > > > > > be > > > > > > dismayed > > > > > > > > > > > > File::Slurp::Tiny and Path::Tiny are both excellent > > > > > > alternatives.

> > > > > > > > > > Thank you. I'll look into them and use alternatives as you > > > > > suggested.

> > > >

Tue Jun 24 10:27:53 2014 dagolden [...] cpan.org - Correspondence added

Look for the discussion of $DYNAMIC_FILE_UPLOAD in HTTP::Request::Common. Supposedly setting that will only read the file in chunks as it uploads, which should avoid OOM problems. Otherwise, you'd need to manage that yourself somehow. E.g. HTTP::Tiny lets you give content as a code reference to return the message body in chunks and you could do your own buffered reads in there. But that's essentially what HTTP::Request is already doing with DYNAMIC_FILE_UPLOAD, I think, so YMMV.

Thu Jun 26 01:17:44 2014 SYAGI [...] cpan.org - Correspondence added

On Tue Jun 24 10:27:53 2014, DAGOLDEN wrote: Show quoted text

> Look for the discussion of $DYNAMIC_FILE_UPLOAD in > HTTP::Request::Common. Supposedly setting that will only read the > file in chunks as it uploads, which should avoid OOM problems. > > Otherwise, you'd need to manage that yourself somehow. E.g. > HTTP::Tiny lets you give content as a code reference to return the > message body in chunks and you could do your own buffered reads in > there. But that's essentially what HTTP::Request is already doing > with DYNAMIC_FILE_UPLOAD, I think, so YMMV.

Thanks for the tip. Looks like DYNAMIC_FILE_UPLOAD will work, at least for the deploy_artifact API call. I just have to change the method signature to expect the filename rather than the file content (otherwise slurping would have to take place on the caller side and there's no way for me to take advantage of dynamic_file_upload).

Fri Jun 27 00:47:13 2014 SYAGI [...] cpan.org - Correspondence added

I spoke too soon. Here is the current situation on this. With $DYNAMIC_FILE_UPLOAD enabled, file upload via PUT request with multipart/form-data works as far as the REST API returning 200 OK. However, the API doesn't really process multipart properly and the artifact content contains the boundary information such as Content-Type: multipart/form-data; boundary="6G+f" --6G+f Content-Disposition: form-data; name="name" which means the API renders the artifact useless. Now, the Artifactory UI is doing some magic to upload the file. The UI is using POST request (as opposed to PUT for REST API) and using multipart/form-data header. I'm not sure if it goes through the REST API underneath but apparently the UI works. So I'm asking jfrog if there's a way to emulate what the UI does on the API. On Thu Jun 26 01:17:44 2014, SYAGI wrote: Show quoted text

> On Tue Jun 24 10:27:53 2014, DAGOLDEN wrote:

> > Look for the discussion of $DYNAMIC_FILE_UPLOAD in > > HTTP::Request::Common. Supposedly setting that will only read the > > file in chunks as it uploads, which should avoid OOM problems. > > > > Otherwise, you'd need to manage that yourself somehow. E.g. > > HTTP::Tiny lets you give content as a code reference to return the > > message body in chunks and you could do your own buffered reads in > > there. But that's essentially what HTTP::Request is already doing > > with DYNAMIC_FILE_UPLOAD, I think, so YMMV.

> > Thanks for the tip. > > Looks like DYNAMIC_FILE_UPLOAD will work, at least for the > deploy_artifact API call. I just have to change the method signature > to expect the filename rather than the file content (otherwise > slurping would have to take place on the caller side and there's no > way for me to take advantage of dynamic_file_upload).

Fri Jun 27 00:48:38 2014 SYAGI [...] cpan.org - Correspondence added

Incidentally I will lose access to pro-version of Artifactory next Wednesday so I am hoping to get this wrapped up by then. On Fri Jun 27 00:47:13 2014, SYAGI wrote: Show quoted text

> I spoke too soon. Here is the current situation on this. > > With $DYNAMIC_FILE_UPLOAD enabled, file upload via PUT request with > multipart/form-data works as far as the REST API returning 200 OK. > However, the API doesn't really process multipart properly and the > artifact content contains the boundary information such as > > Content-Type: multipart/form-data; boundary="6G+f" > > --6G+f > Content-Disposition: form-data; name="name" > > which means the API renders the artifact useless. > > Now, the Artifactory UI is doing some magic to upload the file. The > UI is using POST request (as opposed to PUT for REST API) and using > multipart/form-data header. I'm not sure if it goes through the REST > API underneath but apparently the UI works. So I'm asking jfrog if > there's a way to emulate what the UI does on the API. > > On Thu Jun 26 01:17:44 2014, SYAGI wrote:

> > On Tue Jun 24 10:27:53 2014, DAGOLDEN wrote:

> > > Look for the discussion of $DYNAMIC_FILE_UPLOAD in > > > HTTP::Request::Common. Supposedly setting that will only read the > > > file in chunks as it uploads, which should avoid OOM problems. > > > > > > Otherwise, you'd need to manage that yourself somehow. E.g. > > > HTTP::Tiny lets you give content as a code reference to return the > > > message body in chunks and you could do your own buffered reads in > > > there. But that's essentially what HTTP::Request is already doing > > > with DYNAMIC_FILE_UPLOAD, I think, so YMMV.

> > > > Thanks for the tip. > > > > Looks like DYNAMIC_FILE_UPLOAD will work, at least for the > > deploy_artifact API call. I just have to change the method signature > > to expect the filename rather than the file content (otherwise > > slurping would have to take place on the caller side and there's no > > way for me to take advantage of dynamic_file_upload).

Mon Jun 30 23:09:56 2014 SYAGI [...] cpan.org - Correspondence added

Found a module called HTTP::Request::StreamingUpload, which seems to read the file in chunks and upload without reading in memory. Let me give it a spin and see how it goes. On Fri Jun 27 00:48:38 2014, SYAGI wrote: Show quoted text

> Incidentally I will lose access to pro-version of Artifactory next > Wednesday so I am hoping to get this wrapped up by then. > > On Fri Jun 27 00:47:13 2014, SYAGI wrote:

> > I spoke too soon. Here is the current situation on this. > > > > With $DYNAMIC_FILE_UPLOAD enabled, file upload via PUT request with > > multipart/form-data works as far as the REST API returning 200 OK. > > However, the API doesn't really process multipart properly and the > > artifact content contains the boundary information such as > > > > Content-Type: multipart/form-data; boundary="6G+f" > > > > --6G+f > > Content-Disposition: form-data; name="name" > > > > which means the API renders the artifact useless. > > > > Now, the Artifactory UI is doing some magic to upload the file. The > > UI is using POST request (as opposed to PUT for REST API) and using > > multipart/form-data header. I'm not sure if it goes through the REST > > API underneath but apparently the UI works. So I'm asking jfrog if > > there's a way to emulate what the UI does on the API. > > > > On Thu Jun 26 01:17:44 2014, SYAGI wrote:

> > > On Tue Jun 24 10:27:53 2014, DAGOLDEN wrote:

> > > > Look for the discussion of $DYNAMIC_FILE_UPLOAD in > > > > HTTP::Request::Common. Supposedly setting that will only read > > > > the > > > > file in chunks as it uploads, which should avoid OOM problems. > > > > > > > > Otherwise, you'd need to manage that yourself somehow. E.g. > > > > HTTP::Tiny lets you give content as a code reference to return > > > > the > > > > message body in chunks and you could do your own buffered reads > > > > in > > > > there. But that's essentially what HTTP::Request is already > > > > doing > > > > with DYNAMIC_FILE_UPLOAD, I think, so YMMV.

> > > > > > Thanks for the tip. > > > > > > Looks like DYNAMIC_FILE_UPLOAD will work, at least for the > > > deploy_artifact API call. I just have to change the method > > > signature > > > to expect the filename rather than the file content (otherwise > > > slurping would have to take place on the caller side and there's no > > > way for me to take advantage of dynamic_file_upload).

Tue Jul 01 22:13:33 2014 SYAGI [...] cpan.org - Correspondence added

OK, uploaded Artifactory-Client-v0.7.4 that should address the out of memory issue. I am using the HTTP::Request::StreamingUpload to construct a request object that has the ability to stream a file, instead of reading everything in memory. Note that method signature for deploy_artifact* have changed to take a filename rather than file content in a scalar. Pass in ( file => $file ) to supply the filename. On Mon Jun 30 23:09:56 2014, SYAGI wrote: Show quoted text

> Found a module called HTTP::Request::StreamingUpload, which seems to > read the file in chunks and upload without reading in memory. Let me > give it a spin and see how it goes. > > On Fri Jun 27 00:48:38 2014, SYAGI wrote:

> > Incidentally I will lose access to pro-version of Artifactory next > > Wednesday so I am hoping to get this wrapped up by then. > > > > On Fri Jun 27 00:47:13 2014, SYAGI wrote:

> > > I spoke too soon. Here is the current situation on this. > > > > > > With $DYNAMIC_FILE_UPLOAD enabled, file upload via PUT request with > > > multipart/form-data works as far as the REST API returning 200 OK. > > > However, the API doesn't really process multipart properly and the > > > artifact content contains the boundary information such as > > > > > > Content-Type: multipart/form-data; boundary="6G+f" > > > > > > --6G+f > > > Content-Disposition: form-data; name="name" > > > > > > which means the API renders the artifact useless. > > > > > > Now, the Artifactory UI is doing some magic to upload the file. > > > The > > > UI is using POST request (as opposed to PUT for REST API) and using > > > multipart/form-data header. I'm not sure if it goes through the > > > REST > > > API underneath but apparently the UI works. So I'm asking jfrog if > > > there's a way to emulate what the UI does on the API. > > > > > > On Thu Jun 26 01:17:44 2014, SYAGI wrote:

> > > > On Tue Jun 24 10:27:53 2014, DAGOLDEN wrote:

> > > > > Look for the discussion of $DYNAMIC_FILE_UPLOAD in > > > > > HTTP::Request::Common. Supposedly setting that will only read > > > > > the > > > > > file in chunks as it uploads, which should avoid OOM problems. > > > > > > > > > > Otherwise, you'd need to manage that yourself somehow. E.g. > > > > > HTTP::Tiny lets you give content as a code reference to return > > > > > the > > > > > message body in chunks and you could do your own buffered reads > > > > > in > > > > > there. But that's essentially what HTTP::Request is already > > > > > doing > > > > > with DYNAMIC_FILE_UPLOAD, I think, so YMMV.

> > > > > > > > Thanks for the tip. > > > > > > > > Looks like DYNAMIC_FILE_UPLOAD will work, at least for the > > > > deploy_artifact API call. I just have to change the method > > > > signature > > > > to expect the filename rather than the file content (otherwise > > > > slurping would have to take place on the caller side and there's > > > > no > > > > way for me to take advantage of dynamic_file_upload).

Tue Jul 01 22:13:35 2014 SYAGI [...] cpan.org - Status changed from 'open' to 'resolved'