player/command: add env property

[N-R-K]

mainly so that env vars can be accessed from within input.conf.

[kasper93]

I don't think it's a good idea to allow environmental variables snooping. We already allow clipboard snooping and it isn't quite liked feature, see #17367.

[verygoodlee]

I don't think it's a good idea to allow environmental variables snooping. We already allow clipboard snooping and it isn't quite liked feature, see #17367.

Yes, and this feature can be implemented by a simple lua script, it's not worth adding a property for it.

add this script, and then you can access environment variable through ${user-data/env/...} in input.conf

local utils = require('mp.utils')

local env = {}
for _, v in ipairs(utils.get_env_list()) do
    local i = v:find('=', 1, true)
    env[v:sub(1, i - 1)] = v:sub(i + 1)
end
mp.set_property_native('user-data/env', env)

[N-R-K]

We already allow clipboard snooping and it isn't quite liked feature

This isn't even remotely comparable.

1. Clipboard is shared across processes, env is process local.
2. The clipboard feature isn't liked because it's perma snooping, not due to the feature existing. This property doesn't perma "snoop", not that would make any difference anyways due to point 1.
3. Clipboard snooping breaks password managers doing "one time paste", there's no such thing to break here due to point 1.
4. It's already possible to "snoop" env vars, (see: utils.get_env_list() and verygoodlee's comment) this just gives an easy way to access them in {input,mpv}.conf in order to reference variables like $XDG_RUNTIME_DIR and others.

[guidocella]

You can easily read environment variable in scripts of course, but config files are more convenient for users. (The above script doesn't work for mpv.conf).

Notably there are no ~~ paths for ~/.local/share and $XDG_RUNTIME_DIR.

This has been requested in #mpv a few times and the implementation is trivial so I think it's a fine addition.

[kasper93]

I don't think unpredictable config behavior based on current runtime env is a good thing. But there seems to be strong need for this features, so it's fine.

[na-na-hi]

1. env is process local.

4. It's already possible to "snoop" env vars, (see: utils.get_env_list() and verygoodlee's comment) this just gives an easy way to access them in {input,mpv}.conf in order to reference variables like $XDG_RUNTIME_DIR and others.

JSON IPC clients previously cannot get the environment of mpv, now they can even when they belong to processes different from mpv and normally cannot access mpv's process local environment.

[N-R-K]

JSON IPC clients previously cannot get the environment of mpv, now they can even when they belong to processes different from mpv and normally cannot access mpv's process local environment.

{
    "command": {
        "name": "subprocess"
        "playback_only": false,
        "args": [
            "sh",
            "-c",
            "env > /tmp/mpv.env # or send wherever"
        ],
    }
}

Not to mention, you can read other processes' env from /proc as well. Env vars are simply not a "secure" source to begin with. So there's no security issue in "leaking" them.

[Dudemanguy]

LGTM

[Dudemanguy]

As a future exercise, someone can make utils.get_env_list() a wrapper around this property. The format is a little different between the two so we'll leave discussion of that for another time.

Edit: Actually probably not. Turns out that utils.get_env_list() is probably meant to be fed into the subprocess command.

[verygoodlee]

Currently, ${env/Path} is available on Windows, but ${env/PATH} is not available, the reason is that it is defined as Path.

On Windows, the key of environment variables is case-insensitive, PATH and Path are exactly the same variables, there should be no difference.

[N-R-K]

Currently, ${env/Path} is available on Windows, but ${env/PATH} is not available, the reason is that it is defined as Path.
On Windows, the key of environment variables is case-insensitive, PATH and Path are exactly the same variables, there should be no difference.

Making the windows getenv wrapper case insensitive is probably not too hard. It'd "fix" the sub-property case.

But what about the returned table? env["PATH"] obviously won't work since the table lookup isn't case insensitive.

[N-R-K]

@verygoodlee please test #17691 and report back in there.