FAQFAQ   SearchSearch   MemberlistMemberlist   UsergroupsUsergroups   RegisterRegister 
 ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

HttpClient issues
Goto page 1, 2  Next
 
Post new topic   Reply to topic     Forum Index -> Mango
View previous topic :: View next topic  
Author Message
h3r3tic



Joined: 30 Mar 2004
Posts: 261
Location: Torun, Poland

PostPosted: Mon Jan 16, 2006 5:21 pm    Post subject: HttpClient issues Reply with quote

Hello Smile I've been toying around with HttpClient and I've got some suggestions/bug reports:

1. HttpClient uses the default DisplayWriter, which inserts only '\n' to separate lines. AFAICS, "\r\n" should be used. There's an HttpWriter but it's not used by HttpClient and it still uses '\n'.

2. There's more to it in the HttpClient.open function. There are emit() calls with explicit 'CR'. That should be "\r\n" as well.

3. Automatic redirect following should really be optional. I don't really want it in my program and I had to modify Mango Razz

4. This one is the most problematic IMO: there's a test for a 'bogus request' in the HttpClient.open function. It checks if readable == 0. This test is bogus itself Wink It's related to the way reader() works for Sockets: It just reads the first packet of data that recv() returns. The problem is, recv() doesn't return everything at the first call. It's possible, and it's happened to me, that the first received packed contained "HTTP/1.1 200 OK\r\n". What happened ? That line was be skipped, as in the 'skip any blank lines' section. But then input.readable was 0 and the 'bogus request' check triggerred an error. But though the socket may contain more data and not really be bogus Smile If the test is removed, everything seems to work fine.

Ok, that's all for now Very Happy

Thanks,
Tom
Back to top
View user's profile Send private message MSN Messenger
kris



Joined: 27 Mar 2004
Posts: 1494
Location: South Pacific

PostPosted: Mon Jan 16, 2006 7:06 pm    Post subject: Reply with quote

Thanks, Tom.

* I'm a bit rusty on the HTTP spec, so you're probably right about the \r\n thing.

* How would you prefer to control the redirect? Perhaps through a enableRedirect(bool yes) method? Should it be enabled by default?

* The "truncated response" test is a bit dubious. I think it's actually a hangover from before support for socket-timeout was implemented; but it's been so long I just don't recall the details. There was something about certain sites returning garbage instead of a valid response-line ... hmmm. Perhaps it should be something like this:

Code:

if (line.get.length == 0)
    if (socket.hadTimeout)
         ....


That would catch the situation whereby there was no response, and the following responseLine.parse() will throw if the line is garbage. Does that seem reasonable?
Back to top
View user's profile Send private message
kylefurlong



Joined: 16 Nov 2005
Posts: 29

PostPosted: Mon Jan 16, 2006 11:53 pm    Post subject: Reply with quote

Hey Chris!

This class is what I (mainly) would use Mango for. All the things Tom mentioned I have run into as well. Any effort that you could put in on this part of your wonderful library would be appreciated.

Kyle
Back to top
View user's profile Send private message
kris



Joined: 27 Mar 2004
Posts: 1494
Location: South Pacific

PostPosted: Mon Jan 16, 2006 11:58 pm    Post subject: Reply with quote

kylefurlong wrote:
Hey Chris!

This class is what I (mainly) would use Mango for. All the things Tom mentioned I have run into as well. Any effort that you could put in on this part of your wonderful library would be appreciated.

Kyle

Certainly; thanks for the support!
Back to top
View user's profile Send private message
h3r3tic



Joined: 30 Mar 2004
Posts: 261
Location: Torun, Poland

PostPosted: Tue Jan 17, 2006 3:19 am    Post subject: Reply with quote

kris wrote:
* How would you prefer to control the redirect? Perhaps through a enableRedirect(bool yes) method? Should it be enabled by default?

That sounds reasonable. I'd say it should be enabled by default.
By the way, I just checked, and redirects work wrong when using the redirectPost method and switching to GET. That's because the headers from the POST method are directly used there, such as Content-Length, Content-Type. In my case, the server gets a non-zero Content-Length and waits for data that never arrives, because it's not a POST request.


Quote:
(...)That would catch the situation whereby there was no response, and the following responseLine.parse() will throw if the line is garbage. Does that seem reasonable?

Yeah, it works. Just tested.
But of course line.getLength, not line.get.length Wink

Thanks for your support and keep up the good work
Tom
Back to top
View user's profile Send private message MSN Messenger
kris



Joined: 27 Mar 2004
Posts: 1494
Location: South Pacific

PostPosted: Tue Jan 17, 2006 11:40 am    Post subject: Reply with quote

h3r3tic wrote:
kris wrote:
* How would you prefer to control the redirect? Perhaps through a enableRedirect(bool yes) method? Should it be enabled by default?

That sounds reasonable. I'd say it should be enabled by default.
By the way, I just checked, and redirects work wrong when using the redirectPost method and switching to GET. That's because the headers from the POST method are directly used there, such as Content-Length, Content-Type. In my case, the server gets a non-zero Content-Length and waits for data that never arrives, because it's not a POST request.


Quote:
(...)That would catch the situation whereby there was no response, and the following responseLine.parse() will throw if the line is garbage. Does that seem reasonable?

Yeah, it works. Just tested.
But of course line.getLength, not line.get.length Wink

Thanks for your support and keep up the good work
Tom


Thank-you for taking the time on this. I've checked in an update to SVN, which will require a full SVN synch (at least for http, io, text, etc).

Which headers should be removed in that situation? For now, I just cleared all of them except host. BTW, you're welcome to patch these modules if you like (add your name to the file-header) ~ I'm happy to do it and send it back out, but frankly I prefer to share the ownership Wink

(P.S. line.getLength is not valid in SVN head, I recently changed the LineTokenizer into the new Iterator stuff instead).
Back to top
View user's profile Send private message
h3r3tic



Joined: 30 Mar 2004
Posts: 261
Location: Torun, Poland

PostPosted: Tue Jan 17, 2006 12:15 pm    Post subject: Reply with quote

kris wrote:
Thank-you for taking the time on this. I've checked in an update to SVN, which will require a full SVN synch (at least for http, io, text, etc).

No problem. It's open source after all Wink

Quote:
Which headers should be removed in that situation? For now, I just cleared all of them except host.

In a test I had commenced, both Internet Explorer 6.0 and Firefox 1.5 removed only the Content-Type and Content-Length fields.

Quote:
BTW, you're welcome to patch these modules if you like (add your name to the file-header) ~ I'm happy to do it and send it back out, but frankly I prefer to share the ownership Wink

I'll try Very Happy

Quote:
(P.S. line.getLength is not valid in SVN head, I recently changed the LineTokenizer into the new Iterator stuff instead).

Oh ok. So I shall get everything from SVN
Back to top
View user's profile Send private message MSN Messenger
kris



Joined: 27 Mar 2004
Posts: 1494
Location: South Pacific

PostPosted: Tue Jan 17, 2006 12:29 pm    Post subject: Reply with quote

Ack!

I just thought of a problem we'll run into here ~ MutableHeaders does not currently support removal of individual items. It can be changed to do so, but it'll require an API update. One temporary way around this would be to foreach the existing headers and .dup them into another header-set ... sorry about that.
Back to top
View user's profile Send private message
h3r3tic



Joined: 30 Mar 2004
Posts: 261
Location: Torun, Poland

PostPosted: Tue Jan 17, 2006 12:34 pm    Post subject: Reply with quote

kris wrote:
Ack!

I just thought of a problem we'll run into here ~ MutableHeaders does not currently support removal of individual items. It can be changed to do so, but it'll require an API update. One temporary way around this would be to foreach the existing headers and .dup them into another header-set ... sorry about that.


Yeah, I realize that and I've been planning on doing it thru .dup'ing. But I'll see if adding a remove() is going to be very problematic. Just got mango from svn Smile
Back to top
View user's profile Send private message MSN Messenger
h3r3tic



Joined: 30 Mar 2004
Posts: 261
Location: Torun, Poland

PostPosted: Tue Jan 17, 2006 12:42 pm    Post subject: Reply with quote

Hey kris, looks like the head revision in svn is incomplete. My program doesn't compile because SimpleIterator.d is missing:
"mango/text/SimpleIterator.d: module SimpleIterator cannot read file 'mango/text/SimpleIterator.d'" Smile
Back to top
View user's profile Send private message MSN Messenger
kris



Joined: 27 Mar 2004
Posts: 1494
Location: South Pacific

PostPosted: Tue Jan 17, 2006 1:04 pm    Post subject: Reply with quote

h3r3tic wrote:
Hey kris, looks like the head revision in svn is incomplete. My program doesn't compile because SimpleIterator.d is missing:
"mango/text/SimpleIterator.d: module SimpleIterator cannot read file 'mango/text/SimpleIterator.d'" Smile

Fixed Embarassed
Back to top
View user's profile Send private message
kris



Joined: 27 Mar 2004
Posts: 1494
Location: South Pacific

PostPosted: Tue Jan 17, 2006 1:24 pm    Post subject: Reply with quote

I added support for removal of a header, via a remove() method ... the HttpToken & TokenStack are a bit touchy, given that their goal is to avoid memory allocation; so I felt I should bear the pain of fixing that part Crying or Very sad

Now I'll step away, and get back to work.
Back to top
View user's profile Send private message
h3r3tic



Joined: 30 Mar 2004
Posts: 261
Location: Torun, Poland

PostPosted: Tue Jan 17, 2006 1:26 pm    Post subject: Reply with quote

kris wrote:
I added support for removal of a header, via a remove() method ... the HttpToken & TokenStack are a bit touchy, given that their goal is to avoid memory allocation; so I felt I should bear the pain of fixing that part Crying or Very sad

Now I'll step away, and get back to work.


Thanks Smile So I'll fix the rest in a moment or two /* eating at the moment */
Back to top
View user's profile Send private message MSN Messenger
h3r3tic



Joined: 30 Mar 2004
Posts: 261
Location: Torun, Poland

PostPosted: Tue Jan 17, 2006 3:55 pm    Post subject: Reply with quote

Disclaimer: I'm not a HTTP expert myself. I just made it work for my program that used some POSTs and GETs Smile

Here's a diff/patch: http://158.75.59.9/~h3/tmp/httpClientPatch

I've allowed myself to do a few more changes, e.g. modifying the way a query msg is generated - it wasn't separated with '&', then I modified the HttpWriter to put "\r\n" in its endline() func. I made the HttpClient use that writer instead of DisplayWriter.
Another modification: the usage of POST should now be the same as of GET. Earlier, you'd have to manually set the request data, Content-Length and Content-Type. Content-Length will now be set to the query length and Content-Type will be "application/x-www-form-urlencoded".

Hope it works for you Smile
Tom
Back to top
View user's profile Send private message MSN Messenger
kris



Joined: 27 Mar 2004
Posts: 1494
Location: South Pacific

PostPosted: Tue Jan 17, 2006 6:46 pm    Post subject: Reply with quote

Awesome!

I changed the '&' stuff a little, to make it compatible with tokens.remove() and so on. Also added logic to give 'pump' priority over 'query' when doing a Post ~ I assume these are mutually exclusive?

I do like patches Smile
Back to top
View user's profile Send private message
Display posts from previous:   
Post new topic   Reply to topic     Forum Index -> Mango All times are GMT - 6 Hours
Goto page 1, 2  Next
Page 1 of 2

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum


Powered by phpBB © 2001, 2005 phpBB Group