Re: [Bug-wget] Send Content-Length with POST 0 length body

2014-11-04 Thread Matthew Atkinson
On 21/10/14 09:48, Giuseppe Scrivano wrote:
 Matthew Atkinson mutley...@ntlworld.com writes:
 
 On 20/10/14 21:16, Tim Rühsen wrote:
  
 Since Wget does not explicitly support PUT, we may just care for the POST 
 request. Since servers may reject a POST without Content-Length, we are 
 better 
 off with supplying one. Since PUT (and also PATCH) request 'anticipate' a 
 body, 
 we could also care for these.

 Matthew, could you also check for 'put and 'patch' request and send an 
 amended 
 version of the patch. FYI, HTTP PATCH request is rfc5789.

 Attached
 
 thanks pushed now!  Congratulations on your first contribution to wget.
 
 Regards,
 Giuseppe
 

Hi

I don't think this ever got pushed.

Thanks,
Matt



Re: [Bug-wget] Send Content-Length with POST 0 length body

2014-10-23 Thread Matthew Atkinson
On 21/10/14 09:48, Giuseppe Scrivano wrote:

 thanks pushed now!  Congratulations on your first contribution to wget.
 
 Regards,
 Giuseppe
You sure, or am I being a bit thick? My git is pointing at
git://git.savannah.gnu.org/wget.git and not seeing it

Thanks,
Matt



Re: [Bug-wget] retr-symlinks and directories

2014-10-23 Thread Matthew Atkinson
Thanks

I have attached a patch of where I'm at, it seems to work for all the
cases I can think of on my machine, except that a timeout or other
connection error while testing if I can cd to a symlink will
cause it to skip over that file rather than retry. I'm not really sure 
about error handling, and what should be handled where, what is fatal, etc

Should I be calling a higher level function to CWD and PWD, in order to
handle this? 

Alternatively, how should errors be handled from within
ftp_retrieve_list() 

Thanks
Matt
diff --git a/src/ftp-basic.c b/src/ftp-basic.c
index b6e67e2..152e399 100644
--- a/src/ftp-basic.c
+++ b/src/ftp-basic.c
@@ -1213,3 +1213,26 @@ ftp_process_type (const char *params)
   else
 return 'I';
 }
+
+/* Get current working diectory, CWD to dir, and CWD back */
+uerr_t
+try_cwd (int csock, char *dir)
+{
+  char *pwd = NULL;
+  uerr_t err;
+  err = ftp_pwd (csock, pwd);
+  if (err != FTPOK)
+return err;
+  err = ftp_cwd (csock, dir);
+  if (err != FTPOK)
+return err;
+  err = ftp_cwd (csock, pwd);
+  if (err != FTPOK)
+{
+  /* What if any cleanup is necessary here, or just return err rather than
+   * abort? */
+  fd_close (csock);
+  abort ();
+}
+  return err;
+}
diff --git a/src/ftp.c b/src/ftp.c
index 2d54333..c369809 100644
--- a/src/ftp.c
+++ b/src/ftp.c
@@ -2046,7 +2046,30 @@ Already have correct symlink %s - %s\n\n),
   else/* opt.retr_symlinks */
 {
   if (dlthis)
-err = ftp_loop_internal (u, f, con, NULL);
+{
+  err = ftp_loop_internal (u, f, con, NULL);
+  if (err == FTPNSFOD)
+{
+  err = try_cwd (con-csock, f-name);
+  /* CWD to a non directory will return FTPNSFOD
+   * eg if the symlink points to a non readable file
+   * Should we do this on (err != FTPOK || err != FTPNSFOD?)
+   */
+  if (err == FTPRERR || err == FTPSRVERR)
+{
+  logputs (LOG_VERBOSE, \n);
+  logputs (LOG_NOTQUIET, _(\
+Error in server response, closing control connection.\n));
+  fd_close (con-csock);
+  con-csock = -1;
+}
+  if (err == FTPOK)
+/* Does anything else afterwards depend on this being
+ * FT_SYMLINK? */
+f-type = FT_DIRECTORY;
+}
+}
+
 } /* opt.retr_symlinks */
   break;
 case FT_DIRECTORY:
diff --git a/src/ftp.h b/src/ftp.h
index 78b5270..50d4b65 100644
--- a/src/ftp.h
+++ b/src/ftp.h
@@ -73,6 +73,7 @@ uerr_t ftp_list (int, const char *, bool, bool, bool *);
 uerr_t ftp_syst (int, enum stype *, enum ustype *);
 uerr_t ftp_pwd (int, char **);
 uerr_t ftp_size (int, const char *, wgint *);
+uerr_t try_cwd (int, char *);
 
 #ifdef ENABLE_OPIE
 const char *skey_response (int, const char *, const char *);


[Bug-wget] retr-symlinks and directories

2014-10-22 Thread Matthew Atkinson
Hi

I was looking at http://savannah.gnu.org/bugs/?20373 and considering how
to test if a symlink points to a directory. 

Would it be reliable enough to try CWD to it if getting it as a file
fails, and set f-type to FT_DIRECTORY if that succeeds? If so, what is
the safest way to restore the original PWD? And what should be done on
error in this case?

The only call to ftp_cwd() is wrapped in code which seems to be there to
deal with VMS. Does this need to be done in this case, and if so should
this be moved into ftp_cwd() or another function so it can be reused?

Any thoughts?

Thanks



Re: [Bug-wget] Send Content-Length with POST 0 length body

2014-10-21 Thread Matthew Atkinson
On 20/10/14 21:16, Tim Rühsen wrote:
 
 Since Wget does not explicitly support PUT, we may just care for the POST 
 request. Since servers may reject a POST without Content-Length, we are 
 better 
 off with supplying one. Since PUT (and also PATCH) request 'anticipate' a 
 body, 
 we could also care for these.
 
 Matthew, could you also check for 'put and 'patch' request and send an 
 amended 
 version of the patch. FYI, HTTP PATCH request is rfc5789.

Attached
From 0bc85a619677dc8a0313a5596497d42190fb3452 Mon Sep 17 00:00:00 2001
From: Matthew Atkinson mutley...@ntlworld.com
Date: Tue, 21 Oct 2014 09:28:45 +0100
Subject: [PATCH] Always send Content-Length with POST, PUT, PATCH

---
 src/ChangeLog | 6 ++
 src/http.c| 4 
 2 files changed, 10 insertions(+)

diff --git a/src/ChangeLog b/src/ChangeLog
index d5aeca0..0f92db5 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,9 @@
+2014-10-21  Matthew Atkinson  mutley...@ntlworld.com
+
+	* http.c (gethttp): Always send Content-Length header when method is POST,
+	PUT, or PATCH even with no post body, as some servers will reject the
+	request otherwise
+
 2014-10-16  Tim Ruehsen  tim.rueh...@gmx.de
 
 	* url.c (url_parse): little code cleanup
diff --git a/src/http.c b/src/http.c
index 4b99c17..2469852 100644
--- a/src/http.c
+++ b/src/http.c
@@ -1875,6 +1875,10 @@ gethttp (struct url *u, struct http_stat *hs, int *dt, struct url *proxy,
   xstrdup (number_to_static_string (body_data_size)),
   rel_value);
 }
+  else if (strcasecmp (opt.method, post) == 0
+   || strcasecmp (opt.method, put) == 0
+   || strcasecmp (opt.method, patch) == 0)
+request_set_header (req, Content-Length, 0, rel_none);
 }
 
  retry_with_auth:
-- 
1.9.1



Re: [Bug-wget] Send Content-Length with POST 0 length body

2014-10-20 Thread Matthew Atkinson
On 20/10/14 16:26, Giuseppe Scrivano wrote:
 should we do this only for POST requests?  What about doing it in any
 case that !(opt.body_data || opt.body_file)?
 
 Regards,
 Giuseppe

From http://tools.ietf.org/html/rfc7230#section-3.3.2
 A user agent SHOULD send a Content-Length in a request message when
 no Transfer-Encoding is sent and the request method defines a
 meaning for an enclosed payload body.  For example, a Content-Length
 header field is normally sent in a POST request even when the value
 is 0 (indicating an empty payload body).  A user agent SHOULD
 NOT send a Content-Length header field when the request message
 does not contain a payload body and the method semantics do not
 anticipate such a body.

I would think that we should do this for POST and PUT, but nothing
else

Thanks,
Matt



[Bug-wget] Send Content-Length with POST 0 length body

2014-10-19 Thread Matthew Atkinson

Hi

I was looking through the list archives to find something that useful I 
could contribute to wget and get familiar with the code, I have attached 
a patch for the following.


Darshit Shah darnir at gmail.com wrote on 2014-09-05 07:31:34 GMT
The Content-Length Header is expected by the server in many requests,
most prominently in a POST Request. However, currently if a POST
request is sent without any body, Wget does not send any
Content-Length headers. Some servers seem to dislike this behaviour
and respond with a 411 Length Required message.
Patch Wget to always send a Content-Length header when the Request 
Type is POST.


Hopefully I can find something less trivial to add or fix in the future, 
any suggestions, direction or feedback, please let me know.


Thanks,
Matt
diff --git a/src/http.c b/src/http.c
index 4b99c17..e020682 100644
--- a/src/http.c
+++ b/src/http.c
@@ -1875,6 +1875,8 @@ gethttp (struct url *u, struct http_stat *hs, int *dt, struct url *proxy,
   xstrdup (number_to_static_string (body_data_size)),
   rel_value);
 }
+  else if (strcasecmp (opt.method, post) == 0)
+request_set_header (req, Content-Length, 0, rel_none);
 }
 
  retry_with_auth:


Re: [Bug-wget] Send Content-Length with POST 0 length body

2014-10-19 Thread Matthew Atkinson
On 19/10/14 19:34, Tim Rühsen wrote:
 There two little things - well, just kinda organizational work:
 
 1. Please also extend src/ChangeLog and include it in your patch
 2. The maintainers will ask you for a 'git format-patch' output
   If you never worked with that:
   - git commit your change(s)
   - attach the files generated with 'git format-patch -1'

Attached

Thanks,
Matt
From af9326b1665e0e1d3b28ed0b66c6c99d07a88172 Mon Sep 17 00:00:00 2001
From: Matthew Atkinson mutley...@ntlworld.com
Date: Sun, 19 Oct 2014 21:31:36 +0100
Subject: [PATCH] Always send Content-Length if method is post

---
 src/ChangeLog | 5 +
 src/http.c| 2 ++
 2 files changed, 7 insertions(+)

diff --git a/src/ChangeLog b/src/ChangeLog
index 1c4e2d5..447179e 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,8 @@
+2014-10-19  Matthew Atkinson  mutley...@ntlworld.com
+
+	* http.c (gethttp): Always send Content-Length header when method is post,
+	even with no post body, as some servers will reject the request otherwise
+
 2014-05-03  Tim Ruehsen  tim.rueh...@gmx.de
 
 	* retr.c (retrieve_url): fixed memory leak
diff --git a/src/http.c b/src/http.c
index 4b99c17..e020682 100644
--- a/src/http.c
+++ b/src/http.c
@@ -1875,6 +1875,8 @@ gethttp (struct url *u, struct http_stat *hs, int *dt, struct url *proxy,
   xstrdup (number_to_static_string (body_data_size)),
   rel_value);
 }
+  else if (strcasecmp (opt.method, post) == 0)
+request_set_header (req, Content-Length, 0, rel_none);
 }
 
  retry_with_auth:
-- 
1.9.1