OpenStack Object Storage (swift)

wsgi.make_pre_authed_request fails if path arg is None

Bug #1198149 reported by Alistair Coles on 2013-07-05

This bug affects 1 person

Affects		Status	Importance	Assigned to	Milestone
	OpenStack Object Storage (swift)	Fix Released	Undecided	Alistair Coles	OpenStack Object Storage (swift) 1.9.1

Bug Description

swift/common/wsgi.py make_pre_authed_request() fails if path arg is None (default value) because value is not checked before being passed to unquote()

Jul 5 12:39:40 ubuntu proxy-server 'NoneType' object has no attribute 'split': #012Traceback (most recent call last):#012 File "/home/swift/swift/swift/common/middleware/cluster_federation.py", line 272, in __call__#012 resp = self.forward_request(req, container, obj)#012 File "/home/swift/swift/swift/common/middleware/cluster_federation.py", line 232, in forward_request#012 loc_req = make_pre_authed_request(req.environ, swift_source='CF')#012 File "/home/swift/swift/swift/common/wsgi.py", line 437, in make_pre_authed_request#012 newenv = make_pre_authed_env(env, method, path=unquote(path), agent=agent,#012 File "/usr/lib/python2.7/urllib.py", line 1186, in unquote#012 res = s.split('%')#012AttributeError: 'NoneType' object has no attribute 'split'

Alistair Coles (alistair-coles) on 2013-07-05

Changed in swift:
assignee:	nobody → Alistair Coles (alistair-coles)
status:	New → In Progress

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2013-07-05: Fix proposed to swift (master)

Fix proposed to branch: master
Review: https://review.openstack.org/35816

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2013-07-23: Fix merged to swift (master)

Reviewed: https://review.openstack.org/35816
Committed: http://github.com/openstack/swift/commit/27dc73fe0458ae5334c4547d700f912681a4bcfd
Submitter: Jenkins
Branch: master

commit 27dc73fe0458ae5334c4547d700f912681a4bcfd
Author: anc <email address hidden>
Date: Fri Jul 5 13:18:16 2013 +0100

Fix make_pre_authed_request function to not fail when path arg is None.

    The default value for the path arg to the function is None. However, if
    the path arg is not set or is set to None an exception is generated when
    the path value is passed to urllib.unquote(). The function doc states that
    if path is not provided then the value in the env is used, so fix is to
    implement this behaviour.

Change-Id: I085124acb6ef3cecb2375bb97d27996e0c6fd36e
Fixes: bug #1198149

Changed in swift:
status:	In Progress → Fix Committed

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2013-07-24: Fix proposed to swift (feature/ec)

Fix proposed to branch: feature/ec
Review: https://review.openstack.org/38552

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2013-07-24: Fix merged to swift (feature/ec)

Download full text (15.3 KiB)

Reviewed: https://review.openstack.org/38552
Committed: http://github.com/openstack/swift/commit/f3ba55039c4caa91b71b453f2dc1f5310c2ac801
Submitter: Jenkins
Branch: feature/ec

commit f2c3e1af9cf0ad1fbf7753cd2e5caef9bd297cb5
Author: Samuel Merritt <email address hidden>
Date: Mon Jul 22 16:14:59 2013 -0700

Fix bulk's unit tests on Mac OS.

    Turns out if your $TMPDIR is really long
    (e.g. /var/folders/vq/n32yszdn4s76z6l8dxklmwdh0000gn/T/, which is 50
    characters), then the test that drops stuff into $TMPDIR and adds it
    to a tarball and uploads it will fail due to every added file's name
    being too long.

Change-Id: I8fcab2d95091f72b831b906bfc87a36d8be12631

commit 5c1a7871d9173db9fbd855b72a98ecd8ff163800
Author: Newptone <email address hidden>
Date: Sun Jul 21 12:18:24 2013 +0800

Unified format of boolean params in conf files

    In swift conf files, boolean options use different
    format: some use true/false, and some use True/False.
    This patch is aim to using lowcase true/false to unify
    boolean params formats in swift conf files.

Fix Bug #1203421

Change-Id: I3e1bfc6e43231f51e0710aa54869f3774ee896b1

commit 63061e37ed098bf1ad509177484037544eb6b089
Author: Kun Huang <email address hidden>
Date: Tue Jul 23 13:46:51 2013 +0800

Add notes for /srv/node in swift-object-info

    'devices' is set in object-server.conf on each node, not in ring data,
    and the things printed here is just for watching not for running, so
    just leave a note here. (this https://review.openstack.org/#/c/23951/
    is used for running, so just a note is not enough)

mark this commit as bug fixing is because this script is the last place
using /srv/node but not from conf as Chmouel said.

    fixes import change on read_metadata
    fixes bug #885006
    Change-Id: I727ec2d01c093af61fd3895e5701d87ef67cd9ff

commit 13bbe4b7c31b17efbebd91bd01586757600a082b
Author: Michael Barton <email address hidden>
Date: Mon Jul 22 21:12:22 2013 -0700

fix unit tests in 2.6 by using closing(GzipFile)

Python 2.6 doesn't support using GzipFile as a context manager.

Change-Id: Ic12b5a916bc89ae8d4480879723201c1715285af

commit 6384b192b55b823d86078588f22b8bc847954aad
Author: Alex Gaynor <email address hidden>
Date: Mon Jul 22 14:59:30 2013 -0700

Ensure that files are always closed explicitly.

This is needed on Pythons without reference
counting garbage collectors (e.g. PyPy).

Change-Id: Ieb563ace9f65a4ad204b01be32bf7a9d5f226005

commit 0e3103c0dd3690f5e69ec7953c270430d0564d12
Author: Alex Gaynor <email address hidden>
Date: Mon Jul 22 15:27:54 2013 -0700

Corrected a number of style violations in the tests.

Change-Id: Ib5e81ad0476c56cf84d222d67f55b8db3eb0249e

commit fc293a750c8a83ec99d3b462b4689741c1e66a9b
Author: Peter Portante <email address hidden>
Date: Wed Jul 17 17:41:41 2013 -0400

Move the mount checking into DiskFile constructor

    Move the mount checking into the DiskFile object constructor. The
    REPLICATE method does not use the DiskFile object currently, so that
    method st...

Reviewed:  https://review.openstack.org/38552
Committed: http://github.com/openstack/swift/commit/f3ba55039c4caa91b71b453f2dc1f5310c2ac801
Submitter: Jenkins
Branch:    feature/ec

commit f2c3e1af9cf0ad1fbf7753cd2e5caef9bd297cb5
Author: Samuel Merritt <sam@swiftstack.com>
Date:   Mon Jul 22 16:14:59 2013 -0700

Fix bulk's unit tests on Mac OS.
    
    Turns out if your $TMPDIR is really long
    (e.g. /var/folders/vq/n32yszdn4s76z6l8dxklmwdh0000gn/T/, which is 50
    characters), then the test that drops stuff into $TMPDIR and adds it
    to a tarball and uploads it will fail due to every added file's name
    being too long.
    
    Change-Id: I8fcab2d95091f72b831b906bfc87a36d8be12631

commit 5c1a7871d9173db9fbd855b72a98ecd8ff163800
Author: Newptone <xingchao@unitedstack.com>
Date:   Sun Jul 21 12:18:24 2013 +0800

Unified format of boolean params in conf files
    
    In swift conf files, boolean options use different
    format: some use true/false, and some use True/False.
    This patch is aim to using lowcase true/false to unify
    boolean params formats in swift conf files.
    
    Fix Bug #1203421
    
    Change-Id: I3e1bfc6e43231f51e0710aa54869f3774ee896b1

commit 63061e37ed098bf1ad509177484037544eb6b089
Author: Kun Huang <academicgareth@gmail.com>
Date:   Tue Jul 23 13:46:51 2013 +0800

Add notes for /srv/node in swift-object-info
    
    'devices' is set in object-server.conf on each node, not in ring data,
    and the things printed here is just for watching not for running, so
    just leave a note here. (this https://review.openstack.org/#/c/23951/
    is used for running, so just a note is not enough)
    
    mark this commit as bug fixing is because this script is the last place
    using /srv/node but not from conf as Chmouel said.
    
    fixes import change on read_metadata
    fixes bug #885006
    Change-Id: I727ec2d01c093af61fd3895e5701d87ef67cd9ff

commit 13bbe4b7c31b17efbebd91bd01586757600a082b
Author: Michael Barton <mike@weirdlooking.com>
Date:   Mon Jul 22 21:12:22 2013 -0700

fix unit tests in 2.6 by using closing(GzipFile)
    
    Python 2.6 doesn't support using GzipFile as a context manager.
    
    Change-Id: Ic12b5a916bc89ae8d4480879723201c1715285af

commit 6384b192b55b823d86078588f22b8bc847954aad
Author: Alex Gaynor <alex.gaynor@gmail.com>
Date:   Mon Jul 22 14:59:30 2013 -0700

Ensure that files are always closed explicitly.
    
    This is needed on Pythons without reference
    counting garbage collectors (e.g. PyPy).
    
    Change-Id: Ieb563ace9f65a4ad204b01be32bf7a9d5f226005

commit 0e3103c0dd3690f5e69ec7953c270430d0564d12
Author: Alex Gaynor <alex.gaynor@gmail.com>
Date:   Mon Jul 22 15:27:54 2013 -0700

Corrected a number of style violations in the tests.
    
    Change-Id: Ib5e81ad0476c56cf84d222d67f55b8db3eb0249e

commit fc293a750c8a83ec99d3b462b4689741c1e66a9b
Author: Peter Portante <peter.portante@redhat.com>
Date:   Wed Jul 17 17:41:41 2013 -0400

Move the mount checking into DiskFile constructor
    
    Move the mount checking into the DiskFile object constructor. The
    REPLICATE method does not use the DiskFile object currently, so that
    method still has to perform its own mount check.
    
    We also refactored the DiskFile exceptions a bit for a hierarchy.
    
    Change-Id: Idcbb5a912d8c7d956dac96ff523c1cf1163c98a6
    Signed-off-by: Peter Portante <peter.portante@redhat.com>

commit 8faa21d233c8c454fd156a9cb47020d501ca576f
Author: Peter Portante <peter.portante@redhat.com>
Date:   Thu Jul 18 16:06:15 2013 -0400

Remove unused exceptions
    
    Change-Id: Ib9fe1b52733ffa5b919c2a524a4f3ad7baef09a5
    Signed-off-by: Peter Portante <peter.portante@redhat.com>

commit e0535f9bf358d290f7cce6001f3adcee64c9073f
Author: Peter Portante <peter.portante@redhat.com>
Date:   Fri Jul 19 11:34:12 2013 -0400

Remove extra lstat() calls from check_mount
    
    The os.path.exists call performs an lstat, but os.path.ismount already
    performs the same check. However, it performs a separate lstat() call
    to check for a symlink, which we remove as well, cutting the number
    performed in half.
    
    Sample program to be straced for comparison:
    
    from swift.common.constraints import check_mount
    import os
    os.write(1, "Starting\n")
    if check_mount("/", "tmp"):
        os.write(1, "Mounted\n")
    
    Here is the output of a check on a mounted file system (common case)
    using the new method:
    
    ---- strace new ----
    write(1, "Starting\n", 9) = 9
    lstat("/tmp", {st_mode=S_IFDIR|S_ISVTX|0777, st_size=8460, ...}) = 0
    lstat("/tmp/..", {st_mode=S_IFDIR|0555, st_size=4096, ...}) = 0
    write(1, "Mounted\n", 8) = 8
    
    ---- strace old ----
    write(1, "Starting\n", 9) = 9
    stat("/tmp", {st_mode=S_IFDIR|S_ISVTX|0777, st_size=8460, ...}) = 0
    lstat("/tmp", {st_mode=S_IFDIR|S_ISVTX|0777, st_size=8460, ...}) = 0
    lstat("/tmp", {st_mode=S_IFDIR|S_ISVTX|0777, st_size=8460, ...}) = 0
    lstat("/tmp/..", {st_mode=S_IFDIR|0555, st_size=4096, ...}) = 0
    write(1, "Mounted\n", 8) = 8
    
    Change-Id: I027c862a2b7d9ff99d7f61bd43ccc0825dba525c
    Signed-off-by: Peter Portante <peter.portante@redhat.com>

commit b84afc598b188ef6e654c20fd44ba9d1b17ea55e
Author: Pete Zaitcev <zaitcev@kotori.zaitcev.us>
Date:   Sat Jul 20 18:58:40 2013 -0600

Make stale_reads_ok an argument of __init__
    
    Actually, stale_reads_ok is already an argument, but for some
    reason it was not used anywhere in the code. So, just use it.
    We borrow the existing code from the object server, which uses
    setdefault(). We don't really need it, and could simply use
    kwargs['stale_reads_ok']=stale_reads_ok, but let's keep the
    divergencies down.
    
    This was prompted by the desire to document the API of DB Broker
    for the LFS work, and finding that documenting magical assignments
    was a pain.
    
    Change-Id: Id8f83358ad7709f0df826fbc520b3dfba026a2f1

commit c1f8f266d0c465760535d0305d65138199f67daf
Author: Alex Gaynor <alex.gaynor@gmail.com>
Date:   Sat Jul 20 13:44:11 2013 -0700

Ensure that files in tests are closed.
    
    This is needed on Pythons which do not have
    reference counting GCs (e.g. PyPy).
    
    Change-Id: I5a613e832e9a7a149b3e9317c053c3048f34afcb

commit 7cd01a63d340c254116da1ef9937d30048184dbe
Author: Kun Huang <gareth@unitedstack.com>
Date:   Sat Jul 20 09:52:56 2013 +0800

Add 'Z' into isoformat for UTC time
    
    Based on this http://en.wikipedia.org/wiki/ISO_8601#Time_zone_designators.
    A isofomt of time need a 'Z' for UTC time zone, or none for local time zone
    
    request on GET /<version>/<account>/<container>?format=json still output a
    UTC time without 'Z'
    
    fixes bug #1169287
    Change-Id: Ib599b5ec8fd223878ec18df7c1ec8d952fc2630a

commit 0fdad0d9d9e68b00f61171bb2a0dfd840ef5345f
Author: Alex Gaynor <alex.gaynor@gmail.com>
Date:   Fri Jul 19 14:50:37 2013 -0700

Close SQLite cursors when creating functions.
    
    If the cursors are not closed, then when
    create_function is called, if they are not
    GC'd then create_function will fail. On Pythons
    without reference counting (e.g. PyPy) they
    will not be GC'd immediately.
    
    Change-Id: I39210616d323691ccb745149f24430a7a61382ec

commit 91e7e876b54c44e81df9fa10b111fb57d6c92107
Author: Samuel Merritt <sam@swiftstack.com>
Date:   Thu Jul 18 16:33:01 2013 -0700

Accept valid Accept headers in swob.
    
    "Accept: application/xml; charset=UTF-8" is totally valid, and has an
    implicit q (quality) value of 1.0, just the same as "Accept: text/xml"
    does.
    
    Also, you can say things like:
    Accept: text/xml; charset=UTF-8; q=0.9; anglebrackets="are awesome"
    with as many arbitrary extensions as you want.
    
    See RFC 2616 sections 14.1 Accept and 2.2 Basic Rules for details.
    
    Fixes bug 1202453.
    
    Change-Id: I18e6d0ee3fd6f9d889275ee8335e711c729b7171

commit 3b1ee1f6a3425bca7f5ae98e4701b02653596946
Author: Alex Gaynor <alex.gaynor@gmail.com>
Date:   Fri Jul 19 06:53:48 2013 -0700

Remove list comprehension where the result is unused.
    
    Using a list comprehension here makes the code less
    readable, and is also slower (since it has to allocate
    and grow a list that is unused).
    
    Change-Id: I74cc9d2cf2e271a2c45d45cc5ea777189c02eacb

commit 710e4a007f29c7c7589482282ab4588a4b8bb38a
Author: Alex Gaynor <alex.gaynor@gmail.com>
Date:   Thu Jul 18 17:29:14 2013 -0700

Ensure that files are always closed in the tests.
    
    A failure to close files in a timely fassion means
    that data is not necessarily written immediately
    on Pythons which do not use reference counting
    (e.g. PyPy).
    
    Change-Id: I5d363249676032a025a22a67275c2eed3151b264

commit c9de9f2b8da23538b82f23c724b482404d0ea916
Author: Clay Gerrard <clay.gerrard@gmail.com>
Date:   Wed Jul 17 16:32:35 2013 -0700

Forklift the DiskFile interface into it's own module
    
     * new module swift.obj.diskfile
    
    I parameterized two constants from obj.server into the DiskFile's __init__
    
     * DATADIR -> obj_dir
     * DISALLOWED_HEADERS -> disallowed_metadata_keys
    
    I'm not sure if this is the right long term abstraction but for now it avoids
    circular imports.
    
    Change-Id: I3962202c07c4b2fbfc26f9776c8a5c96292ae199

commit 53f502687be1d2e4983c2e6cb1fbf643176af4a1
Author: David Hadas <davidh@il.ibm.com>
Date:   Thu Jul 18 00:00:46 2013 +0300

Fix the way possibly_quarantine reraises an exception
    
    Fixes Bug #1202386
    
    Change-Id: I8585d8442c7ccf538b819b036192b28c1f13847b

commit 9480ff8a28e6a5e0a6b79dc2456381bbb76e9c98
Author: Peter Portante <peter.portante@redhat.com>
Date:   Tue Jun 25 15:16:35 2013 -0400

Refactor auditors to rely on expected gen names
    
    Refactor the various auditors to rely on the audit_location_generator
    yielding tuples containing paths with the expected suffix.
    
    We also fix the exception handling for container_sync to not expect a
    broker object (since the act of creating a broker object can raise an
    exception).
    
    For the object auditor we removed an unneeded check for disk_file
    since get_data_file_size() will raise DiskFileNotExist under the same
    condition (raises code coverage slightly).
    
    Change-Id: I11d405e629063177ef21543b75e9076da1a03b61

commit bc99f58c765b2a710ddb73994f1825206ca5fc66
Author: Peter Portante <peter.portante@redhat.com>
Date:   Mon Jul 1 17:40:47 2013 -0400

Fix unit tests to properly marked deleted files
    
    The unit tests were playing fast and loose with the tombstone marker,
    where the test framework was setting up a DiskFile object which had
    its data written to the .ts file, not the .data file. This behavior
    did not reflect how the interfaces to DiskFile were supposed to
    work.
    
    Change-Id: Idd6e8882e062ba2e13489f14189223ab4158677c
    Signed-off-by: Peter Portante <peter.portante@redhat.com>

commit 18a0813d9b3896159a4ed4819cc15e00a4a9cc03
Author: Chmouel Boudjnah <chmouel@enovance.com>
Date:   Mon Jul 15 17:14:16 2013 +0200

Add documentation about flake8+hacking.
    
    - Fixes bug 1201431.
    
    Change-Id: If025a41caf3a629b9efb4d67c53c423796d37a91

commit 27dc73fe0458ae5334c4547d700f912681a4bcfd
Author: anc <alistair.coles@hp.com>
Date:   Fri Jul 5 13:18:16 2013 +0100

Fix make_pre_authed_request function to not fail when path arg is None.
    
    The default value for the path arg to the function is None. However, if
    the path arg is not set or is set to None an exception is generated when
    the path value is passed to urllib.unquote(). The function doc states that
    if path is not provided then the value in the env is used, so fix is to
    implement this behaviour.
    
    Change-Id: I085124acb6ef3cecb2375bb97d27996e0c6fd36e
    Fixes: bug #1198149

commit 8aba2d602ef913ebdcaebd46510324299dfc3e39
Author: Dirk Mueller <dirk@dmllr.de>
Date:   Thu Jun 20 20:36:59 2013 +0200

Start using Hacking
    
    Instead of blacklisting Hacking globally,
    only blacklist those that currently occur frequently
    (for a later followup patch), and fix the rest. In
    detail:
    
    H101  Use TODO(NAME)
    H201  no 'except:' at least use 'except Exception:'
    H231  octal number 022 should be written as 0o22
    H401  docstring should not start with a space
    H701  Empty localization string
    
    Change-Id: Ib3b3d56b68d1cf15d3b67ac9749fcbdb876dc52a

commit bc08215f839a0845a29e016b74677d56b4808883
Author: Vladimir Vechkanov <vvechkanov@mirantis.com>
Date:   Thu May 23 20:16:21 2013 +0400

Move replication allow method to decorators
    
    Remove logic of allowed methods list from object, container and account
    servers. Instead of it add replicator decorator to utils and use new
    decorator for REPLICATE methods in object/account/container servers.
    This decorator mark method as special for usfor use only by the
    replication.
    
    If the option replication_server is not used, then this mechanism is not
    enabled. If the replicaton_server option is set (not None) then the
    respective server is a replicator (option value is True) and should use
    ONLY the methods marked for replication server using the decorator, or
    it is a normal server type and should NOT use methods marked for the
    replication server.
    
    Change-Id: I1041b31413cd0c39000317cc57a8c27816e1dfe8

commit e7b5e9d44f3442f5ac8717257628739d05d8fdf8
Author: Greg Lange <greglange@gmail.com>
Date:   Tue Jul 9 20:27:52 2013 +0000

make internal client metadata methods return dicts with lower case keys
    
    Change-Id: I19ced30b6e92133766310f8e41286202607c5b4c

commit 6f57b06c7cf34de4d4a842c18d864eecbdc63f23
Author: Chmouel Boudjnah <chmouel@enovance.com>
Date:   Mon Jul 8 13:30:17 2013 +0200

Make keystoneauth reseller_prefix check for _
    
    - Automatically add underscore if there wasn't one already.
    - This makes consistent with tempauth.
    - Fixes bug 1198878.
    - This should probably need to be mentioned in the release changelog for
      upgrades.
    
    Change-Id: Ia82f14f82c380c7ed2ddd190116f1493e410cfbe

commit 6f722f7320c42415407587974ac9100a2cb8d08f
Author: Chmouel Boudjnah <chmouel@enovance.com>
Date:   Wed May 15 10:48:31 2013 +0200

Don't allow users to delete their own account.
    
    - In keystoneauth we allowed authenticated users to delete their own
      account we are disallowing that and only allow users with reseller
      admin to do that for its own or for the others.
    - Fixes bug 1177526.
    
    Change-Id: I825c5a968e8eae0991915056825fe0e0c195647e

commit 1f7d2a60d658935391fbab6cee6d3f37c8f8788e
Author: Vladimir Vechkanov <vvechkanov@mirantis.com>
Date:   Wed May 15 12:58:57 2013 +0400

Refactor and add tests for db_replicator
    
    * Create class for testing _repl_to_not and replicate_object fuctions to
      prevent duplication code by adding all preparation into setUp function.
    * Move existed test function which testin _repl_to_not and
      replicate_object into created classes.
    * Add tests for replicate_object and _repl_to_node functions.
    
    Change-Id: I75ac7c6f0230e71bfb24328e44c33734b520b4cd

commit 885e02102d13735ba0e07f55e64e7ed4a2ba9c50
Author: Samuel Merritt <sam@swiftstack.com>
Date:   Fri Jun 28 11:54:35 2013 -0700

Extract quorum-size calculation to utility method.
    
    The behavior's the same as before; this just makes the code (IMHO) a
    little easier to read.
    
    Change-Id: Ie54d836d81af7410057219f60b72b840c9ce51b9

Thierry Carrez (ttx) on 2013-08-07

Changed in swift:
milestone:	none → 1.9.1
status:	Fix Committed → Fix Released

Report a bug

This report contains Public information

Everyone can see this information.

You are

Subscribing...

Edit bug mail

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.