RE: [PEAR-DEV] Re: [PEAR-CVS] cvs: pear /HTTP_Request Request.php

> Hi! > > moshe doron wrote: > >>My best solution so far (didn't test it) is a regular expression based > > > > split > > > >>on the HTTP/1.1 headers. > >>To my opinion the last item in the returned array should assigned the > >>$response variable where now the socket is read. > >> > >>resulting in something like: (near line 709) > >>$fullRespone = $sock->readAll(); > >>$reponseParts = preg_split("^HTTP/1.[0|1"\ \d{3}\ .*\n$", > > > > $fullResponse, -1, > > > >>PREG_SPLIT_NO_EMPTY); //split on HTTP header > >>$response = array_pop($responseParts); //returning last element > > > > > > here my implementation for the bad HTTP_Response code this > code deal with: > > 1. 100 > > 2. cookies wrong urlencode > > 3. improving the the flow structure by removing duplicate code > into else\if. > > http://moshe.i-com-it.com/patchs/HTTP_Request.3.txt > > > > anyway your implementaion is bad coz we need the headers of > the 100 chunk > > into the 200 (or whatever) one (e.g, times cookies are send on the 100 > > chunk). > > IMO both solutions smell of hack too much. :[ > > What really needs to be done to support 100 Continue and to f.e. > give the > ability to display progress bar when downloading a large file is > refactoring > HTTP_Response, *especially* getting rid of $socket->readAll() > call. As the class > is effectively private, I suspect it will not lead to any BC breaks. > > What I suggest: > Removing response-processing logic from constructor (there can > be errors and > constructors cannot return these). > Creating a readResponse() method with the following logic (pseudocode): > > do { > readHeaders(); > processHeaders(); > notifyListeners(); > if (100 == $code) { > $headers = true; > } else { > $headers = false; > } > } while ($headers); > > while ($data = readData()) { > processData(); > notifyListeners(); > } > > > I uploaded Stefan Walk's version of HTTP_Request I already talked about: > http://oc.cs.msu.su/pear/Request.phps > > I do not agree with his implementation and the amount of changes > he did, but > this can serve as a starting point to extract a smaller and more > manageable patch. > > Your opinions? > First of all, this issue was reported more than three weeks ago. And because i couldn't find any reactions regarding this issue, i just did the kick off ;-) In the patch all headers are stored lowercase. The request class however tries to read the Location header (not lowercase) and theirfor pear bug #18 will raise again But i must say that i like the rest of the implementation. With best regards, Dave Mertens