inconsistent url quoting in x-symlink-target

[clayg]

You can symlink to an object "test space" in your test container using either of these "x-symlink-target" values:

"test/test space"
"test/test%20space"

That's because the symlink middleware makes subrequests without quoting the target_path [1]

The result is that it's difficult and confusing to symlink to an object named "100%farts"

If you create an object named "100% farts" in your test container you symlink with x-symlink-target of "test/100% farts" [2], but if you try the obvious "test/100%farts" you get a 412 [3], because symlinks makes an invalid request - unmodified %fa is a non-utf-8 control character

Instead you have to use x-symlink-target "test/100%25farts"

This strange behavior doesn't really make it impossible to symlink to any object name I've been able to discover, but the optional quoting and the failures on some invalid x-symlink-target values seem incorrect. The testing we have in place to cover this scenario is suspect [4].

1. https://github.com/openstack/swift/blob/fab0a8767e4b4de001dbc6e448e9586f39a1c644/swift/common/middleware/symlink.py#L388
2. "test/100%25%20farts" "test/100%%20farts" and "test/100%25 farts" also work, but that's probably bad
3. https://github.com/openstack/swift/blob/master/swift/proxy/server.py#L469
4.
diff --git a/test/functional/test_symlink.py b/test/functional/test_symlink.py
index 1684f3b05..70319a400 100755
--- a/test/functional/test_symlink.py
+++ b/test/functional/test_symlink.py
@@ -286,6 +286,31 @@ class TestSymlink(Base):

         self.assertEqual(resp.status, 201)

+ wtf = {
+ # this should 200
+ urllib.parse.quote(
+ 'dealde%2Fl04 011e%204c8df/flash.png'): 404,
+ # this should 404
+ urllib.parse.unquote(
+ 'dealde%2Fl04 011e%204c8df/flash.png'): 200,
+ # this is maddness
+ 'dealde%2Fl04 011e%204c8df/flash.png': 200,
+ 'dealde/l04 011e%204c8df/flash.png': 200,
+ 'dealde%2Fl04%20011e%204c8df/flash.png': 200,
+ 'dealde%2Fl04 011e%204c8df%2Fflash.png': 200,
+ 'dealde%2Fl04%20011e%204c8df%2Fflash.png': 200,
+ }
+ failures = []
+ for name, status in wtf.items():
+ resp = retry(
+ self._make_request, method='GET',
+ container=self.env.tgt_cont, obj=name)
+ if resp.status != status:
+ failures.append('%r returned %s, expected %s' % (
+ name, resp.status, status))
+ self.assertFalse(failures, 'some paths did not work:\n' +
+ '\n'.join(failures))
+
         # PUT symlink
         self._test_put_symlink(link_cont=self.env.link_cont, link_obj=link_obj,
                                tgt_cont=self.env.tgt_cont,

[OpenStack Infra (hudson-openstack)]

Change abandoned by Clay Gerrard (<email address hidden>) on branch: master
Review: https://review.openstack.org/645345
Reason: merge Change-Id: I1f300d69bec43f0deb430294da05a4ec04308040
 instead

[clayg (clay-gerrard)]

Fix is to normalized the clients x-symlink-target values with quote(unquote(orig))

https://review.openstack.org/#/c/571906/4

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/571906
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=e5e22ebeba2140b1da2e9d52d6b8ecd92bc75c88
Submitter: Zuul
Branch: master

commit e5e22ebeba2140b1da2e9d52d6b8ecd92bc75c88
Author: Tim Burke <email address hidden>
Date: Fri Jun 1 15:38:10 2018 -0700

    Make symlink work with Unicode account names

    Also, ensure that the stored symlink headers really *are* url-encoded as
    we've been assuming.

    Change-Id: I1f300d69bec43f0deb430294da05a4ec04308040
    Related-Bug: 1774238
    Closes-Bug: #1821240

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: feature/losf
Review: https://review.opendev.org/655630

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/655630
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=d391957fd70a02866615c9c5edc070dbde2b23ea
Submitter: Zuul
Branch: feature/losf

commit 32bf43990c3c9ce771fca5968242f102155467c1
Author: ZhongShengping <[e-mail]>
Date: Mon Apr 22 11:05:10 2019 +0800

    Replace git.openstack.org URLs with opendev.org URLs

    Thorough replacement of git.openstack.org URLs with their opendev.org
    counterparts.

    Change-Id: I5e77307de6a484fd69b2a5863423ceb357e8601f

commit 89eced960c5bf5c2e14b6245c70b615dc23d45a6
Author: OpenDev Sysadmins <email address hidden>
Date: Fri Apr 19 19:28:47 2019 +0000

    OpenDev Migration Patch

    This commit was bulk generated and pushed by the OpenDev sysadmins
    as a part of the Git hosting and code review systems migration
    detailed in these mailing list posts:

    http://lists.openstack.org/pipermail/openstack-discuss/2019-March/003603.html
    http://lists.openstack.org/pipermail/openstack-discuss/2019-April/004920.html

    Attempts have been made to correct repository namespaces and
    hostnames based on simple pattern matching, but it's possible some
    were updated incorrectly or missed entirely. Please reach out to us
    via the contact information listed at https://opendev.org/ with any
    questions you may have.

commit b6ebabee78b77108b2a3e5261873f038772d304d
Author: Tim Burke <email address hidden>
Date: Wed Apr 17 09:15:27 2019 -0700

    Clean up dlo unit tests

    We don't actually need so many py2/py3 branches, and once we
    clean those up, there's hardly any reason to import six.

    Change-Id: Ia3b4f02e7eb99ad1a76aa35c39dc198528fd39ad

commit 893acffbc01ba817361f8d8735513dc784fb92c0
Author: Pete Zaitcev <email address hidden>
Date: Wed Feb 20 17:09:08 2019 -0600

    py3: port dlo

    Change-Id: I7236ddea0acde93d0789ad8affa76df0097a86aa

commit 7cb276da425592280cd3f522c06cfb7b02311bde
Author: Tim Burke <email address hidden>
Date: Tue Apr 2 17:42:21 2019 -0700

    Move from py35 to py37 for gating py3 jobs

    Looking at https://governance.openstack.org/tc/reference/runtimes/train.html
    OpenStack is no longer targetting Python 3.5, so it doesn't really make sense
    to gate on it. Move to Python 3.7, just to make it so we can go longer before
    having to do this shuffle again.

    Presumably, once we actually declare support for Python 3.6, we should
    make that job voting, too? We've had enough gate flaky-ness lately that
    I'm not keen on it.

    Change-Id: I9c9e5df76a6592ab9c512b7c1ba807f9a0053059

commit 65d3cb75f817b3683aa85b73239d606e29c73d1c
Author: zhufl <email address hidden>
Date: Mon Apr 15 15:12:30 2019 +0800

    Use assertIn to check whether substring is in str

    This is to use assertIn to check whether substring is in str,
    go get clearer failure message.

    Change-Id: I40aff29454c423389755a5330751d2f69a227a05

commit e5ff405ec967bfe19b3eaeaac5b5a32c95218f9d
Author: Tim Burke <email address hidden>
Date: Fri Jun 1 15:59:25 2018 -0700

    Make staticweb return URL-encoded Location headers

 ...

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/swift 2.22.0 release.