Skip to content

Commit

Permalink
Order of hashes no longer canonical (#110)
Browse files Browse the repository at this point in the history
Fixes #105 

CC @SomberNight

### Here's how I tested it:

First observe that for `requests` the sha256 digests appear in this order:

1. `7bf2a778576d825600030a110f3c0e3e8edc51dfaafe1c146e39a2027784957b`
2. `502a824f31acdacb3a35b6690b5fbf0bc41d63a24a45c4004352b0242707598e`

<details>
▶ curl https://pypi.org/pypi/requests/json | jq '.releases["2.21.0"]'
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  107k  100  107k    0     0   483k      0 --:--:-- --:--:-- --:--:--  485k
[
  {
    "comment_text": "",
    "digests": {
      "md5": "ed3af234ffcad0b3c1e521e1dfde19be",
      "sha256": "7bf2a778576d825600030a110f3c0e3e8edc51dfaafe1c146e39a2027784957b"
    },
    "downloads": -1,
    "filename": "requests-2.21.0-py2.py3-none-any.whl",
    "has_sig": false,
    "md5_digest": "ed3af234ffcad0b3c1e521e1dfde19be",
    "packagetype": "bdist_wheel",
    "python_version": "py2.py3",
    "requires_python": ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*",
    "size": 57987,
    "upload_time": "2018-12-10T15:40:08",
    "url": "https://files.pythonhosted.org/packages/7d/e3/20f3d364d6c8e5d2353c72a67778eb189176f08e873c9900e10c0287b84b/requests-2.21.0-py2.py3-none-any.whl"
  },
  {
    "comment_text": "",
    "digests": {
      "md5": "1bcd0e0977c3f8db1848ba0e2b7ab904",
      "sha256": "502a824f31acdacb3a35b6690b5fbf0bc41d63a24a45c4004352b0242707598e"
    },
    "downloads": -1,
    "filename": "requests-2.21.0.tar.gz",
    "has_sig": false,
    "md5_digest": "1bcd0e0977c3f8db1848ba0e2b7ab904",
    "packagetype": "sdist",
    "python_version": "source",
    "requires_python": ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*",
    "size": 111528,
    "upload_time": "2018-12-10T15:40:11",
    "url": "https://files.pythonhosted.org/packages/52/2c/514e4ac25da2b08ca5a464c50463682126385c4272c18193876e91f4bc38/requests-2.21.0.tar.gz"
  }
]
</details>

Run it the first time:

```
▶ python hashin.py -r /tmp/reqs.txt requests

▶ cat /tmp/reqs.txt
requests==2.21.0 \
    --hash=sha256:502a824f31acdacb3a35b6690b5fbf0bc41d63a24a45c4004352b0242707598e \
    --hash=sha256:7bf2a778576d825600030a110f3c0e3e8edc51dfaafe1c146e39a2027784957b
```
Note that `502a824...` comes before `7bf2a778...` which is different from the JSON from Pypi.org. 

Next, mess with the list of hashes manually:

```
▶ jed /tmp/reqs.txt

▶ cat /tmp/reqs.txt
requests==2.21.0 \
    --hash=sha256:7bf2a778576d825600030a110f3c0e3e8edc51dfaafe1c146e39a2027784957b \
    --hash=sha256:502a824f31acdacb3a35b6690b5fbf0bc41d63a24a45c4004352b0242707598e

▶ python hashin.py -r /tmp/reqs.txt requests --dry-run

```

In other words, it didn't touch the file. It's because when it compares the previous hashes with the found hashes from pypi, it does it by comparing to `set`s. 

Now really mess with it:

```
▶ cat /tmp/reqs.txt
requests==2.21.0 \
    --hash=sha256:7bf2a778576d825600030a110f3c0e3e8edc51dfaafe1c146e39a2027784957b \
    --hash=sha256:502a824f31acdacb3a35b6690b5fbf0bc41d63a24a45c4004352b0242707598e

▶ jed /tmp/reqs.txt

▶ cat /tmp/reqs.txt
requests==2.21.0 \
    --hash=sha256:7bf2a778576d825600030a110f3c0e3e8edc51dfaafe1c146e39a2027784957b

▶ python hashin.py -r /tmp/reqs.txt requests

▶ cat /tmp/reqs.txt
requests==2.21.0 \
    --hash=sha256:502a824f31acdacb3a35b6690b5fbf0bc41d63a24a45c4004352b0242707598e \
    --hash=sha256:7bf2a778576d825600030a110f3c0e3e8edc51dfaafe1c146e39a2027784957b
```

This time, the sets aren't equal so it goes to write down the hashes and when it does it does so in a sorted order.
  • Loading branch information
Peter Bengtsson authored Jan 31, 2019
1 parent 90f2804 commit eed8a17
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 5 deletions.
2 changes: 1 addition & 1 deletion hashin.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ def run_packages(
maybe_restriction = "" if not restriction else "; {0}".format(restriction)
new_lines = "{0}=={1}{2} \\\n".format(req, data["version"], maybe_restriction)
padding = " " * 4
for i, release in enumerate(data["hashes"]):
for i, release in enumerate(sorted(data["hashes"], key=lambda r: r["hash"])):
new_lines += "{0}--hash={1}:{2}".format(padding, algorithm, release["hash"])
if i != len(data["hashes"]) - 1:
new_lines += " \\"
Expand Down
61 changes: 57 additions & 4 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ def test_amend_requirements_content_replacement_2():
assert result == new_lines[2]


def test_amend_requirements_content_replacement_amonst_others():
def test_amend_requirements_content_replacement_amongst_others():
previous = (
"""
otherpackage==1.0.0 --hash=sha256:cHay6ATFKumO3svU3B-8qBMYb-f1_dYlR4OgClWntEI
Expand Down Expand Up @@ -489,7 +489,7 @@ def test_amend_requirements_content_replacement_amonst_others():
assert result == previous + new_lines[2]


def test_amend_requirements_content_replacement_amonst_others_2():
def test_amend_requirements_content_replacement_amongst_others_2():
previous = (
"https://github.com/rhelmer/pyinotify/archive/9ff352f.zip"
"#egg=pyinotify "
Expand Down Expand Up @@ -609,7 +609,6 @@ def mocked_get(url, **options):
# Now check the verbose output
captured = capsys.readouterr()
out_lines = captured.out.splitlines()
# out_lines = my_stdout.getvalue().splitlines()
assert "https://pypi.org/pypi/hashin/json" in out_lines[0], out_lines[0]
# url to download
assert "hashin-0.10-py2-none-any.whl" in out_lines[1], out_lines[1]
Expand Down Expand Up @@ -647,6 +646,60 @@ def mocked_get(url, **options):
) in lines


def test_canonical_list_of_hashes(murlopen, tmpfile, capsys):
"""When hashes are written down, after the package spec, write down the hashes
in a canonical way. Essentially, in lexicographic order. But when comparing
existing hashes ignore any order.
"""

def mocked_get(url, **options):
if url == "https://pypi.org/pypi/hashin/json":
return _Response(
{
"info": {"version": "0.10", "name": "hashin"},
"releases": {
# NOTE that the order of list is different from
# the order you'd get of `sorted(x.digest for x in releases)`
"0.10": [
{
"url": "https://pypi.org/packages/source/p/hashin/hashin-0.10.tar.gz",
"digests": {"sha256": "ccccc"},
},
{
"url": "https://pypi.org/packages/2.7/p/hashin/hashin-0.10-py2-none-any.whl",
"digests": {"sha256": "aaaaa"},
},
{
"url": "https://pypi.org/packages/3.3/p/hashin/hashin-0.10-py3-none-any.whl",
"digests": {"sha256": "bbbbb"},
},
]
},
}
)
raise NotImplementedError(url)

murlopen.side_effect = mocked_get

with tmpfile() as filename:
with open(filename, "w") as f:
f.write("")

retcode = hashin.run("hashin==0.10", filename, "sha256")

assert retcode == 0
with open(filename) as f:
output = f.read()
assert output
assert output.endswith("\n")
lines = output.splitlines()

assert lines[0] == "hashin==0.10 \\"
assert lines[1] == " --hash=sha256:aaaaa \\"
assert lines[2] == " --hash=sha256:bbbbb \\"
assert lines[3] == " --hash=sha256:ccccc"


def test_run_atomic_not_write_with_error_on_last_package(murlopen, tmpfile):
def mocked_get(url, **options):

Expand Down Expand Up @@ -1623,7 +1676,7 @@ def mocked_get(url, **options):


def test_run_dry(murlopen, tmpfile, capsys):
"""dry run should edit the requirements.txt file and print
"""dry run should not edit the requirements file and print
hashes and package name in the console
"""

Expand Down

0 comments on commit eed8a17

Please sign in to comment.