oasis-open / cti-taxii-client

OASIS TC Open Repository: TAXII 2 Client Library Written in Python

https://taxii2client.readthedocs.io/

BSD 3-Clause "New" or "Revised" License

107 stars 51 forks source link

Fix "Range" header according to TAXII version 2.0 documenation #82

Closed teizenman closed 3 years ago

teizenman commented 3 years ago

We are using TAXII2.0 servers that started failing lately, after the merge of the following PR: https://github.com/oasis-open/cti-taxii-client/pull/75

According to the documentation of TAXII2.0 the Range header should not have the = prior to the range desired.

Link to the part in the documentation stating this: http://docs.oasis-open.org/cti/taxii/v2.0/cs01/taxii-v2.0-cs01.html#_Toc496542716

codecov-commenter commented 3 years ago

Codecov Report

Merging #82 into master will decrease coverage by 0.38%. The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master      #82      +/-   ##
==========================================
- Coverage   94.89%   94.51%   -0.39%     
==========================================
  Files           8        8              
  Lines        1784     1658     -126     
==========================================
- Hits         1693     1567     -126     
  Misses         91       91

Impacted Files	Coverage Δ
taxii2client/v20/__init__.py	`89.36% <0.00%> (-0.78%)`	:arrow_down:
taxii2client/v21/__init__.py	`90.85% <0.00%> (-0.70%)`	:arrow_down:
taxii2client/test/test_client_v20.py	`100.00% <0.00%> (ø)`
taxii2client/test/test_client_v21.py	`100.00% <0.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 6a3f4f0...8c5e8ac. Read the comment docs.

emmanvg commented 3 years ago

@teizenman, we believe this was due to an error when the spec was written for TAXII 2.0. They cited in that section RFC 7233 which defines Range (Section 3) with the use of "=".

Another supporting article: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Range

teizenman commented 3 years ago

Hi @emmanvg and thank you for the quick reply.

I want through those files, and it seems like the = sign is required when units are being used, which as far as I understood it's not the case here.

In addition, I'm adding two screenshots from the project while debugging the request being sent. The screenshots are taken from common.py line 296

With the = sign I get <Response [400]>
Without the = sign I get <Response [206]> as expected

emmanvg commented 3 years ago

Again, RCF7233 defines other range units with the use of = similar to the other supported documents. In the TAXII use-case items is our Range <unit>.

If you are using Medallion, the server returns a 400 as a malformed request. It could be changed to:

drop the header in request, process with rest of information and return 200 OK (server MAY ignore Range header according to RFC7233)
a different status code maybe 416? (unlikely)
Drop the header and start from the beginning since it could not interpret the Range Header.
Update both Medallion and the Client to the latest versions.

I don't have enough information to think the problem is here.

teizenman commented 3 years ago

@emmanvg Would you like us to have a zoom session regarding this issue?

emmanvg commented 3 years ago

@teizenman Sure. Can you include @clenk when you send the invite.

emmanvg commented 3 years ago

For the record. Thanks for reporting and providing a fix for this issue 🎉! In order for me to merge your changes you will need to sign an Individual CLA at OASIS. Signed CLAs can be viewed here. Given that you decided not to sign in the interest of time, I will be authoring these changes and closing this PR. You should see the changes in the next release.