Provide request body in request info. #811
Conversation
|
Oops I broke some tests, will reopen when I fix it. |
Codecov Report
@@ Coverage Diff @@
## master #811 +/- ##
==========================================
- Coverage 88.9% 88.89% -0.02%
==========================================
Files 41 41
Lines 5670 5726 +56
Branches 781 791 +10
==========================================
+ Hits 5041 5090 +49
- Misses 459 462 +3
- Partials 170 174 +4
|
|
|
||
| Note that request data in :ref:`splash-request-info` is not available in the | ||
| callback :ref:`splash-on-response-headers` or in the request of the response | ||
| returned by :ref:`splash-http-get` and :ref:`splash-http-post`. |
There was a problem hiding this comment.
Where is it available then? You mentioned HAR, is it available in on_request callbacks?
There was a problem hiding this comment.
Yes, and also in on_response (in response.request.info).
I was going to add a test for that now, but I hit what I believe to be a bug, which I reported as #814
There was a problem hiding this comment.
Now it also has a test for on_response.
splash/network_manager.py
Outdated
| # WebCore::FormDataIODevice object, which is sequential. Its | ||
| # getFormDataSize() method cannot be accessed through PyQt5, | ||
| # but Qt WebKit puts its value in the Content-Length header | ||
| # (see WebCore::QNetworkReplyHandler::getIODevice). |
splash/network_manager.py
Outdated
| @@ -123,6 +123,38 @@ def createRequest(self, operation, request, outgoingData=None): | |||
| self.log(traceback.format_exc(), min_level=1, format_msg=False) | |||
| return super(ProxiedQNetworkAccessManager, self).createRequest(operation, request, outgoingData) | |||
|
|
|||
| def _get_request_body(self, request, outgoing_data): | |||
| if outgoing_data is not None: | |||
There was a problem hiding this comment.
It is often easier to read functions when special cases are handled first:
if outgoing_data is None:
return None
# ... the restthis also provides more horizontal space (less nesting).
splash/network_manager.py
Outdated
| if outgoing_data.isSequential(): | ||
| # In a sequential QIODevice, size() returns the value of | ||
| # bytesAvailable(), which is only the size of the data in the | ||
| # QIODevice buffer and not the total size of the output. Until |
There was a problem hiding this comment.
Do you know if tests trigger this behavior (data with a size larger than a buffer)? It seems new tests only use very small request bodies.
There was a problem hiding this comment.
Good point. I added a test for it.
… when outgoing_data is None.
|
Looks great, thanks @ArturGaspar! |
Fixes #688
Besides the issues mentioned in the added documentation (non-standard HAR, request body not available in certain Lua functions), when this functionality is enabled it breaks the HAR viewer, which always expects postData with type "application/x-www-form-urlencoded" to have the "params" field.