Witold Filipczyk <[EMAIL PROTECTED]> writes:

> The big_files-1008-r branch is derived from master. On that branch
> there is the code handling big file uploads.

Thank you.  It is easier for me to review a branch than a set of
patches.

In addition to what I posted as comment #4 in bug 1008, I see
these potential problems:

- 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.

- It is not clear how http_connection_info.post_fd is closed if
  the user aborts the connection.  Is http_end_request() called
  in that case?  It seems not.

- send_big_files() should check whether the open(big_file + 1,
  O_RDONLY) call succeeds.

- If something changes the size of the file while ELinks is
  sending it, then it seems possible that the amount of data
  posted does not match the generated Content-Length header.
  ELinks should detect this and report an error instead of
  annoying the server with an invalid request.  (Alternatively,
  send the POST request in chunked encoding, so that the size
  need not be known in advance.)

- You moved struct http_connection_info from
  src/protocol/http/http.c to src/protocol/http/http.h but left
  the related macros and comments in http.c.  Please keep them
  together, or at least add a comment in the definition of the
  structure, saying that the values of those macros are used.

- In struct uri, there's a comment /* Number of files bigger than
  1M */ but the actual limit in encode_multipart() seems to be 64k.

- In encode_multipart(), please check for error from fstat().

- Please move struct big_files_offset into src/viewer/text/form.c
  and add a comment saying that the begin and end numbers
  indicate the position of the file name in the POST data
  generated by encode_multipart(), thus int will be large enough
  regardless of how big the files are.

- Please fully document the format of uri.post, including:
  - whether it contains only the body or can also contain headers;
  - that most of the data is encoded in hexadecimal;
  - that the data can include names of files to be posted, and
    each such name begins and ends with BIG_FILE_CHAR;
  - that BIG_FILE_CHAR has this meaning only in the post component,
    i.e. when it follows POST_CHAR.

Attachment: pgpJi5hVV2t6k.pgp
Description: PGP signature

_______________________________________________
elinks-dev mailing list
[email protected]
http://linuxfromscratch.org/mailman/listinfo/elinks-dev

Reply via email to