Add Cookie parser for http requests#6
Open
mepahoo wants to merge 29 commits into
Open
Conversation
Mordor has a very useful HTTP library, but it tends to require a complete knowledge of the code and HTTP spec/terminology to make even a simple client request. So far I had always just cut and paste existing mordor code and then tried to adapt it blindly but it was usually a recipe for frustation. As part of my code jam time I've done a study of part of the code and added more comments. I tried to only document things that I'm confident about, but please review, as a bad comment can be more misleading than no comment at all. http.h - Try to establish the relationship between the structures there and the actual HTTP headers that the client and server need to pass over the wire. BTW it seems to be an oversight that there is no direct Cookie support? I added a comment with the workaround uri.h - Maybe I'm the only dev unfamiliar with all the detailed terminology for the parts of an URI, but I thought a few examples would clarify what scheme, authority and fragment mean. Also I found it is not clear how to properly build a query for an URI, and there is a risk of accidental double encoding/decoding if you don't ues it right. So I put some specific tips on that subject. broker.h - RequestBrokers are a critical part of making a connection but the abstraction and flexibility also makes it a bit obscure. Another challenge is the fact that this header file exposes all sorts of details that a normal user of the library wouldn't need to know. This is a first pass at describing each class and some of the important methods, with some emphasis on the ConnnectionCache. (Note: now that I understand it a bit better I think this code is very cool) Change-Id: If90f1f571248d03392894b16e3208f01d3eebc92 Reviewed-on: https://gerrit.dechocorp.com/20605 Reviewed-by: Jenkins <hudsonbuild@mozy.com> Reviewed-by: Jeremy Stanley <jeremy@mozy.com> Reviewed-by: Dan Nuffer <dann@mozy.com>
The call to evaluate a PAC script was crashing in the underlying JavaScript VM: JSC::Heap::markConservatively(JSC::MarkStack&, void*, void*) The JS garbage collector does not work properly on a fiber-based thread, so the solution was to isolate the call into a fiber-free thread. The worker thread utilizes a message queue to communicate with the requesting fiber-based thread. The fiber uses Mordor::sleep while waiting to allow other fibers to continue. Change-Id: If6a661bce5b320ed518f5183767ce8d7d1df31ef Reviewed-on: https://gerrit.dechocorp.com/20915 Reviewed-by: Jenkins <hudsonbuild@mozy.com> Reviewed-by: Andrew Skowronski <andrews@mozy.com>
The ProxyCache is typically used on every single http request to look up the proxy rules for each url. However autodetection has been found to take 15-30 seconds to fail on some of our computers, which makes sync completely unusable. Autodetection is controlled in the Internet Options in Control panel but the option is hard to find and seems to be on by default on some machines. So a fix in Mordor also seemed prudent. Rather than always calling WinHttpGetProxyForUrl this fix now calls the more explicit WinHttpDetectAutoProxyConfigUrl to perform the autodetection. If that fails it does not attempt again during the lifetype of the ProxyCache, which for Kalypso and Sync is the same as the lifetime of the RequestBroker. The downside of this extra caching is the client needs to do extra work to reset the cache if the network configuration changes. e.g. if you move your laptop to a corporate network then an existing RequestBroker would not pick up the need for a proxy unless ProxyCache::resetDetectionResultCache is called. fixes #790 (https://labs.dechocorp.com/issues/790) Change-Id: I962e0fde6c5e54d812379662e38caf34b213684c Reviewed-on: https://gerrit.dechocorp.com/21069 Reviewed-by: Jenkins <hudsonbuild@mozy.com> Reviewed-by: Jeremy Stanley <jeremy@mozy.com>
Ignore autogenerated xcuserdata Change-Id: I8bc91093bee4b314d578c987756d88880a3e1357 Reviewed-on: https://gerrit.dechocorp.com/21156 Reviewed-by: Jenkins <hudsonbuild@mozy.com> Reviewed-by: Ben Dolman <bend@mozy.com>
Signed-off-by: Wilson Pan <weiminp@mozy.com> Change-Id: I7750a67a9559ffcacbd6042ac536bb7b19f0b61e Reviewed-on: https://gerrit.dechocorp.com/21201 Reviewed-by: Jenkins <hudsonbuild@mozy.com> Reviewed-by: Kevin Cai <kevinc@mozy.com>
Use a separate output directory for each example so they don't get sharing violations during parallel compilation. Similar to tethys fix gerrit/21168 Change-Id: Ie9dec6f55dcfada4d07e0e02eadd2abaf99bc110 Reviewed-on: https://gerrit.dechocorp.com/21400 Reviewed-by: Jenkins <hudsonbuild@mozy.com> Reviewed-by: Jeremy Stanley <jeremy@mozy.com>
current format is BINARY only. However this may not suit every situation.
A typical scenario is this:
when calling a stored procedure which returns void, the result format has to be TEXT.
A fatal_error will be returned if the result format is BINARY.
There are a couple of stored procedures in demeter returning void. So this is kind of
a blocker to trogdor, which uses mordor to access demeter.
with this patch, BINARY is still the default format. While you can call
conn->prepare("...", "", PreparedStatement::TEXT) to specify the result format.
Signed-off-by: Dannis Song <danniss@mozy.com>
Change-Id: I22d49ef5e67ab946b82092c2146ff541b4c9576d
Reviewed-on: https://gerrit.dechocorp.com/21430
Reviewed-by: Jenkins <hudsonbuild@mozy.com>
Reviewed-by: Wilson <weiminp@mozy.com>
Parser was crashing if attribute value contained references, e.g. " or ' For example a=""" or b=''' (but a="'" is legal xml) The existing regular expression seemed to being trying to call a separate callback for each reference, as is done for the inner text. I was not able to fix that and in any case it would be problematic for a parser to handle the same callback invoked for both building inner text and attribute values without more context info. Instead I demonstrate in the unit test how the attribute value callback could take care of converting any references itself. I also added a bit more general unit testing. I found that comments are not supported, but fixing that is beyond my ragel skills so i just leave a unit test as a reminder. Change-Id: I7c5f735269eabf6b80a123c6f3e0ff746713e180 Change-Id: Iec17341a32858c3d86b78140bc2c20383ed5c00f Reviewed-on: https://gerrit.dechocorp.com/21366 Reviewed-by: Jenkins <hudsonbuild@mozy.com> Reviewed-by: Jeremy Stanley <jeremy@mozy.com> Reviewed-by: Rodi Reich <rodir@mozy.com>
Problem statement: * Client has to be very carefully using the timer, has to remember to cancel the time in destructor combining with exception handling * Under current implementation, Timer object is not 100% cancellable after the timer enters the expiring queue * So there are a bit possiblity that when timer is timed out and the callback is executed, the dependent objects are already destroyed (Especially, using class member function as the timer callback, and the object itself is detroyed, so the 'this' point is invalid) Sulotion: * Leverage weak_ptr to detect expired 'this' object Implementation: * registerConditonTimer interface is added Limitation: * By using weak_ptr, the timer can't be registered in object constructor Change-Id: I50b91be01a68bdeea1b71d2227cc56c92b073012 Signed-off-by: Kevin Cai <kevinc@mozy.com> Reviewed-on: https://gerrit.dechocorp.com/21354 Reviewed-by: Jenkins <hudsonbuild@mozy.com> Reviewed-by: Jeremy Stanley <jeremy@mozy.com>
…td::string's -use stringicmp for CookieKey (domain and name should be insensitive) -fix problem in CookieRequestParser state machine
TritonClient was attempting to flush requestStream in its error handling. If the stream is in an error state then just throw again, following similar logic to other methods in ClientRequest. Note: when the assert was ignored, e.g. in release build, the eventual flush() call would end up throwing a ConnectionAbortedException from the socket library. So an alternative fix would be to adjust the assert and let the error case proceed along its old code path. Reference: https://labs.dechocorp.com/issues/816 Change-Id: I75b69274828d535cc6012e4da4c9d985a3190b98 Reviewed-on: https://gerrit.dechocorp.com/21692 Reviewed-by: Jenkins <hudsonbuild@mozy.com> Reviewed-by: Jeremy Stanley <jeremy@mozy.com>
Change-Id: I4bcff0a4f0534acb58b8a9721eeaa6cea63827c4 Signed-off-by: Kefu Chai <chaik@mozy.com> Reviewed-on: https://gerrit.dechocorp.com/21783 Reviewed-by: Kevin Cai <kevinc@mozy.com> Reviewed-by: Jenkins <hudsonbuild@mozy.com> Reviewed-by: Jeremy Stanley <jeremy@mozy.com>
as these functions are often broken by buggy third-party Layered Service Providers (LPSs). also fix a few bugs in the fallback paths, and make the Socket::accept(Socket &) function private, since it has never worked (there's no way to construct a Socket with m_sock == -1 without using the private ctor that Socket::accept() makes use of) Change-Id: Ib6c830c05ce7a6e031e240852615d7b5c750674e Reviewed-on: https://gerrit.dechocorp.com/21925 Reviewed-by: Andrew Skowronski <andrews@mozy.com> Reviewed-by: Jenkins <hudsonbuild@mozy.com>
* with a zero-size stream, ServerRequest::responseDone will be called twice, first for notifyOnEof(), second for notifyOnClose() which will cause the same ServerRequest been commit twice which crashes the ServerConnection. * Fix it by checking the response stream size, DO NOT transferStream to responseStream() if size is zero. Change-Id: I3cbe631b198e4dd5c76d3cf0cc81c283f3a0d393 Signed-off-by: Kevin Cai <kevinc@mozy.com> Reviewed-on: https://gerrit.dechocorp.com/22552 Reviewed-by: Jenkins <hudsonbuild@mozy.com> Reviewed-by: Dannis Song <danniss@mozy.com> Reviewed-by: Andrew Skowronski <andrews@mozy.com> Reviewed-by: Russ Simons <russs@mozy.com>
If * the original API returns -1 * SSL_get_error() returns SSL_ERROR_SYSCALL * the SSL error queue is empty (!hasOpenSSLError()) then it means the underlying BIO has failed, and the system error code should be set. Rather than doing MORDOR_NOTREACHED (which will terminate the application), throw an exception with the last-error code. Change-Id: I7b3840303b911e04ae7fcfa48beabbf40f56c023 Reviewed-on: https://gerrit.dechocorp.com/22791 Reviewed-by: Jenkins <hudsonbuild@mozy.com> Reviewed-by: Andrew Skowronski <andrews@mozy.com>
Conflicts: mordor/streams/ssl.cpp
…7c6e66d367d0c99d)
This method allows to execute multiple statements at once without prepare overhead
… text format results
Conflicts: mordor/http/http.h
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
-added CookieKey struct that acts as the key to find cookies in a CookieList
-added Cookie struct that contains cookie data
-added CookieList that is a map of cookies
-added CookieRequestParser that can parse a cookie header into a CookieLsist
--added stringicmp to string for case insensitive compares between 2 std::strings (helps CookieKey)