readthedocs / readthedocs.org

The source code that powers readthedocs.org
https://readthedocs.org/
MIT License
7.99k stars 3.58k forks source link

Environment variable values sometimes contain single quotes #8636

Open chrisbouchard opened 2 years ago

chrisbouchard commented 2 years ago

Details

I created an environment variable LINKCODE_URL_FORMAT with the value https://github.com/chrisbouchard/easyrepr/blob/{git_hash}/{filename}#L{start_line}-L{end_line}. When I get the value using os.environ['LINKCODE_URL_FORMAT'] in my Sphynx docs/conf.py, the value is wrapped in single quotes. This does not happen when I set the environment variable's value to something simple like abc123. I'm very sure I didn't include the quotes when I created the variable as I recreated it several times.

My suspicion is that the {} characters are triggering shell quoting in whatever script runs conf.py, but that it's getting quoted twice. I do not have this problem when I build my docs locally, which makes me think it's not a bug in Sphynx or the makefile.

Expected Result

os.environ['LINKCODE_URL_FORMAT'] == "https://github.com/chrisbouchard/easyrepr/blob/{git_hash}/{filename}#L{start_line}-L{end_line}"

Actual Result

os.environ['LINKCODE_URL_FORMAT'] == "'https://github.com/chrisbouchard/easyrepr/blob/{git_hash}/{filename}#L{start_line}-L{end_line}'"
stsewd commented 2 years ago

Looks like this is coming from https://github.com/readthedocs/readthedocs.org/blob/f38fe0f48ed4fcfa715f647bbd073356effbb9ee/readthedocs/projects/models.py#L1881-L1881

But I'm not sure if that's needed, as we aren't concatenating those values manually or anything unsafe as far as I see https://github.com/readthedocs/readthedocs.org/blob/f38fe0f48ed4fcfa715f647bbd073356effbb9ee/readthedocs/doc_builder/environments.py#L305-L305

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

humitos commented 2 years ago

@stsewd were you able to replicate this issue?

stsewd commented 2 years ago

yes, the problem is explained at https://github.com/readthedocs/readthedocs.org/issues/8636#issuecomment-956379776, we are using quote basically.

12rambau commented 1 year ago

Just to note that this makes the way readthedocs deals with env variable inconsistent with other major cervices that I use, namely: Github, GitLab and Circle. The variable I wanted to use in #10553 is a json string, that I need to write back in a file. It works everywhere but here, I would have never managed to debug it without @humidos guidances.

douglas-raillard-arm commented 2 weeks ago

Just hit the issue as well, for an env var that contains a JSON value. This breaks the json decoding as a result.

EDIT: My current workaround that removes extra quotation if there is any:

import shlex
def unquote(v):
    try:
        _v = shlex.split(v)
    # Not quoted in the first place
    except ValueError:
        return v
    else:
        try:
            v, = _v
        # More than one item, this means the string was not quoted
        except ValueError:
            return v
        else:
            return v

vals = ['a', 'a b', '1', '2 |']

for v in vals:
    v2 = unquote(v)
    v3 = unquote(shlex.quote(v))
    assert v2 == v
    assert v3 == v