Re: [elinks-dev] Big files upload
On Mon, May 19, 2008 at 08:30:15AM +0300, Kalle Olavi Niemitalo wrote: > Kalle Olavi Niemitalo <[EMAIL PROTECTED]> writes: > > > Witold Filipczyk <[EMAIL PROTECTED]> writes: > > > >> On Mon, May 12, 2008 at 12:14:52AM +0300, Kalle Olavi Niemitalo wrote: > >>> - The code is duplicated between src/protocol/file/cgi.c and > >>> src/protocol/http/http.c. This may be the best way but it > >>> looks a bit annoying. > >> > >> I have no idea how to make it better. > > > > How about the following? I did not test this yet. > > I tested file and bigtextarea POSTs with CGI, and a file upload > at http://validator.w3.org/. > There was a bug in http_read_post_data_inline(). > I amended the commit to fix that, and pushed. > > uri->post can begin with a Content-Type string and '\n'. > http_send_header() in http.c and send_post_data() in cgi.c check > whether this is the case. It seems theoretically possible that > in a POST without a Content-Type, a file name could contain '\n' > and confuse this check. Likewise, if a file name contains > FILE_CHAR, it will confuse http_read_post_data_inline(). To fix > those, I think we should either change encode_multipart() to fail > if a file name contains one of those characters, or change the > format of uri->post so that it can be unambiguously parsed. I'd > prefer changing the format because then we could also put the > lengths of the files there and check that they match. I doubt that one can enter the filename with a newline into the input field. > upload_stat_timer() in src/network/connection.c assumes that > conn->info is struct http_connection_info * whenever > conn->upload_progress != NULL. We could document this at the > definition of connection.upload_progress but I think it would be > nicer to move the upload counters into struct connection so that > non-HTTP protocols can show upload progress too. Just remember > to clear the counters if the connection is reused. > > The progress counters should be off_t rather than size_t so that > they can support >4GiB files on 32-bit systems. (Or perhaps even > longlong because off_t can be 32-bit and multiple files may be > uploaded in one request. That would require larger changes > though.) Do whatever you want. The condition is that it must work. -- Witek ___ elinks-dev mailing list elinks-dev@linuxfromscratch.org http://linuxfromscratch.org/mailman/listinfo/elinks-dev
Re: [elinks-dev] Big files upload
Witold Filipczyk <[EMAIL PROTECTED]> writes: > (b) is not acceptable. > I chose /dev/urandom. Okay but please add error handling and a check that fread() got all the bytes it wanted. E.g. on Windows, there will be neither /dev/urandom nor /dev/prandom, and ELinks should detect this rather than proceed with an uninitialized and perhaps not very random buffer. I see though that encode_multipart() does not properly handle errors from e.g. add_char_to_string() either. Anyway, it's better to keep the bug in one function than spread it around. pgpwG1c8XUekA.pgp Description: PGP signature ___ elinks-dev mailing list elinks-dev@linuxfromscratch.org http://linuxfromscratch.org/mailman/listinfo/elinks-dev
Re: [elinks-dev] Big files upload
Kalle Olavi Niemitalo <[EMAIL PROTECTED]> writes: > Witold Filipczyk <[EMAIL PROTECTED]> writes: > >> On Mon, May 12, 2008 at 12:14:52AM +0300, Kalle Olavi Niemitalo wrote: >>> - The code is duplicated between src/protocol/file/cgi.c and >>> src/protocol/http/http.c. This may be the best way but it >>> looks a bit annoying. >> >> I have no idea how to make it better. > > How about the following? I did not test this yet. I tested file and bigtextarea POSTs with CGI, and a file upload at http://validator.w3.org/. There was a bug in http_read_post_data_inline(). I amended the commit to fix that, and pushed. uri->post can begin with a Content-Type string and '\n'. http_send_header() in http.c and send_post_data() in cgi.c check whether this is the case. It seems theoretically possible that in a POST without a Content-Type, a file name could contain '\n' and confuse this check. Likewise, if a file name contains FILE_CHAR, it will confuse http_read_post_data_inline(). To fix those, I think we should either change encode_multipart() to fail if a file name contains one of those characters, or change the format of uri->post so that it can be unambiguously parsed. I'd prefer changing the format because then we could also put the lengths of the files there and check that they match. upload_stat_timer() in src/network/connection.c assumes that conn->info is struct http_connection_info * whenever conn->upload_progress != NULL. We could document this at the definition of connection.upload_progress but I think it would be nicer to move the upload counters into struct connection so that non-HTTP protocols can show upload progress too. Just remember to clear the counters if the connection is reused. The progress counters should be off_t rather than size_t so that they can support >4GiB files on 32-bit systems. (Or perhaps even longlong because off_t can be 32-bit and multiple files may be uploaded in one request. That would require larger changes though.) pgp9yotDHOAXn.pgp Description: PGP signature ___ elinks-dev mailing list elinks-dev@linuxfromscratch.org http://linuxfromscratch.org/mailman/listinfo/elinks-dev
