Project

Profile

Help

Refactor #3303

v2.py: Accept header handling is sketchy

Added by mihai.ibanescu@gmail.com almost 4 years ago. Updated about 2 years ago.

Status:
CLOSED - CURRENTRELEASE
Priority:
Normal
Start date:
Due date:
% Done:

100%

Estimated time:
Platform Release:
2.15.1
Target Release - Crane:
Groomed:
No
Sprint Candidate:
No
Tags:
Pulp 2
Sprint:
Quarter:

Description

Docker engines typically present more than one Accept: header (usually three).

While they are three different lines in the HTTP header, they typically get combined as a single value, comma-separated.

Crane has a bunch of if/then/else logic that looks like:

accept_headers = request.headers.get('Accept')
schema2_mediatype = 'application/vnd.docker.distribution.manifest.v2+json'
manifest_list_mediatype = 'application/vnd.docker.distribution.manifest.list.v2+json'
if manifest_list_mediatype in accept_headers ...:
   ...

That is essentially the equivalent of evaluating:

"val1" in "val1, val2, val3"

and boils down to string matching.

I find it dangerous, because "val" in "val1, val2, val3" is also True.

It just so happens that the Docker engine is civilized enough to present the right values.

A much better solution would be something like:

accept_headers = accept_headers.split(",") if accept_headers else []
accept_headers = set(x.strip() for x in accept_headers)

Associated revisions

Revision cf972a36 View on GitHub
Added by Mihai Ibanescu almost 4 years ago

Safer handling of Accept headers

closes #3303 https://pulp.plan.io/issues/3303

Revision 0553c802 View on GitHub
Added by Mihai Ibanescu almost 4 years ago

Safer handling of Accept headers

closes #3303 https://pulp.plan.io/issues/3303

(cherry picked from commit cf972a363b8c25323a1e53c831749537268f0a3a)

History

#2 Updated by mihai.ibanescu@gmail.com almost 4 years ago

  • Description updated (diff)

#3 Updated by dkliban@redhat.com almost 4 years ago

  • Status changed from NEW to POST
  • Assignee set to mihai.ibanescu@gmail.com

#4 Updated by dalley almost 4 years ago

  • Tracker changed from Issue to Refactor
  • % Done set to 0

#5 Updated by Anonymous almost 4 years ago

  • Status changed from POST to MODIFIED
  • % Done changed from 0 to 100

#6 Updated by pcreech almost 4 years ago

  • Platform Release set to 2.15.1

#7 Updated by pcreech almost 4 years ago

  • Status changed from MODIFIED to 5

#8 Updated by pcreech almost 4 years ago

  • Status changed from 5 to CLOSED - CURRENTRELEASE

#9 Updated by bmbouter over 2 years ago

  • Tags Pulp 2 added

Please register to edit this issue

Also available in: Atom PDF