Bugzilla

(In reply to zhchbin from comment #12)
> I have another suggestion for fixing this problem. Maybe we can check if the default filename contains `%` before passing it to the file picker, if yes, call win32 api `ExpandEnvironmentStringsW` to expand environment-variable strings. After that we can replace illegal characters correctly.

Sure that's also a possibility. I was mostly discussing with Molly about where to apply the fix, and we seem to agree fixing at the filepicker implementation level makes more sense, because it catches more cases and it's more future proof.

Whether we should "nullify" the variables or translate them, was not decided yet.
One downside of expanding the ENV vars (or better asking the system to do it), is that the user may still not understand the risk of saving into certain system folders, while disabling the vars would generate an ugly file name but would prevent a misunderstandment. Not sure if this is a blocker though, since the site could still use social engineering to convince the user to save into certain folders, it would just be harder than with automatic var expansion.

(In reply to zhchbin from comment #12)
> I have another suggestion for fixing this problem. Maybe we can check if the default filename contains `%` before passing it to the file picker, if yes, call win32 api `ExpandEnvironmentStringsW` to expand environment-variable strings. After that we can replace illegal characters correctly.

Sure that's also a possibility. I was mostly discussing with Molly about where to apply the fix, and we seem to agree fixing at the filepicker implementation level makes more sense, because it catches more cases and it's more future proof.

Whether we should "nullify" the variables or translate them, was not decided yet.
One downside of expanding the ENV vars (or better asking the system to do it), is that the user may still not understand the risk of saving into certain system folders, while disabling the vars would generate an ugly file name but would prevent a misunderstandment. Not sure if this is a blocker though, since the site could still use social engineering to convince the user to save into certain folders, it would just be harder than with automatic var expansion.

The other downside is that our code sanitizes before passing to the file picker, thus sanitizing after var expansion would not allow to apply the fix to the file picker (unless we sanitize twice).

(In reply to zhchbin from comment #12)
> I have another suggestion for fixing this problem. Maybe we can check if the default filename contains `%` before passing it to the file picker, if yes, call win32 api `ExpandEnvironmentStringsW` to expand environment-variable strings. After that we can replace illegal characters correctly.

Sure that's also a possibility. I was mostly discussing with Molly about where to apply the fix, and we seem to agree fixing at the filepicker implementation level makes more sense, because it catches more cases and it's more future proof.

Whether we should "nullify" the variables or translate them, was not decided yet.
One downside of expanding the ENV vars (or better asking the system to do it), is that the user may still not understand the risk of saving into certain system folders, while disabling the vars would generate an ugly file name but would prevent a misunderstandment. Not sure if this is a blocker though, since the site could still use social engineering to convince the user to save into certain folders, it would just be harder than with automatic var expansion.

The other downside is that our code sanitizes before passing to the file picker, thus sanitizing after var expansion would not allow to apply the fix in the file picker code (unless we sanitize twice).

(In reply to zhchbin from comment #12)
> I have another suggestion for fixing this problem. Maybe we can check if the default filename contains `%` before passing it to the file picker, if yes, call win32 api `ExpandEnvironmentStringsW` to expand environment-variable strings. After that we can replace illegal characters correctly.

Sure that's also a possibility. I was mostly discussing with Molly about where to apply the fix, and we seem to agree fixing at the filepicker implementation level makes more sense, because it catches more cases and it's more future proof.

Whether we should "nullify" the variables or translate them, was not decided yet.
One downside of expanding the ENV vars (or better asking the system to do it), is that the user may still not understand the risk of saving into certain system folders, while disabling the vars would generate an ugly file name but would prevent a misunderstandment. Not sure if this is a blocker though, since the site could still use social engineering to convince the user to save into certain folders, it would just be harder than with automatic var expansion.

The other downside is that our code sanitizes before passing to the file picker, thus sanitizing after var expansion would not allow to easily apply the fix to the file picker code (unless we expand, sanitize, pass to the picker, sanitize vars again to prevent future misuse).