search

snyk-labs / pysnyk

A Python client for the Snyk API.
https://snyk.docs.apiary.io/
MIT License
85 stars 116 forks source link

[BUG]: Improper Handling of Version and Limit Parameters on Get Rest Pages in v0.9.12 #194

Closed ig596 closed 12 months ago

ig596 commented 1 year ago

Is there an existing issue for this?

Description of the bug

This commit appears to have adjusted the behavior of get_rest_pages(), resulting in appending duplicate query params that result in an invalid query. ?limit=100&version=experimental?limit=100&version=experimental&

Steps To Reproduce

Run the following code on an organization containing a large number of projects

import snyk
org_id="xxxxxx"
snyk_token="xxxxx"
snyk_client=snyk.SnykClient(token=snyk_token,
            delay=60,
            backoff=1,
            tries=3,
            version="experimental",
            url="https://api.snyk.io/rest/",
            debug=True)
projects=snyk_client.get_rest_pages(f"/orgs/{org_id}/projects", params={"limit": 100})

Additional Information

Version Experiencing Bug

[[package]]
name = "pysnyk"
version = "0.9.12"
description = "A Python client for the Snyk API"
optional = false
python-versions = ">=3.7,<4.0"
files = [
    {file = "pysnyk-0.9.12-py3-none-any.whl", hash = "sha256:aa1c2d6ee1664548942072eeb6c1221ee9e4e76eb0341631b5c54f9efc64cb34"},
    {file = "pysnyk-0.9.12.tar.gz", hash = "sha256:28515236ec4e63e0146e6a0eec82437aa89a48182e5d9832f799f80931584db0"},
]
ig596 commented 12 months ago

@nathan-roys thanks for the prompt efforts. I believe this is still a bug. We now notice that if the client is instantiated as done in the reproduction code above, this updated release will duplicate /rest/ in the path and also not properly include a ? for the query params and only uses & between them leading to incorrect parsing. If you are unable to address this please let me know where in the code I can look to try to fix for a PR.

nathan-roys commented 12 months ago

@ig596 I'll look in to this & get back to you next week :)

nathan-roys commented 12 months ago

More than happy if you'd like to take a look though @ig596, I'm happy to look at PRs.

If you look at my previous PR, you can see the relevant code: https://github.com/snyk-labs/pysnyk/pull/195/files

ig596 commented 12 months ago

@nathan-roys I was wrong. We had an issue with our cache and lock file not properly updating. 0.9.13 fixed the original issue.