pp.c/pp_hot.c - prevent aliasing or localizing read only value in a restricted hash #20963
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
| a general comment - that first commit also makes minor cosmetic changes to the warning messages, which generates large amounts of trivial diffs which swamp out the main purpose of the commit. It would be better to split off into two commits. |
Yeah, ill split it out. The diag tests weren't picking up the hv_notallowed() error messages and there were various inconsistencies that were annoying me so I cleaned it up. I'll split that out. |
The diff view is intended for very long strings where it is difficult to eyeball what changed. Showing it for short strings like stringified refs is not that helpful, so raise the threshold from 20 chars to 60 chars.
demerphq force-pushed the yves/restricted_hash_disallow_alias_or_local branch from 41fe7f5 to c17464c Compare
@iabyn i have split them up as requested. |
demerphq force-pushed the yves/restricted_hash_disallow_alias_or_local branch 2 times, most recently from b571f32 to 2f01a2c Compare Use a standardized wording for restricted hash errors, thus instead of saying "from a restricted hash" and "in a restricted hash", say "in restricted hash" consistently. This will be exploited in a subsequent patch more than it is now. Modify hv_allowed() to take a new parameter showing the action that is not allowed. This will be used in a subsequent patch to reduce the code involved in cases where the action may vary depending on a parameter. Use SVf_QUOTEDPREFIX not SVf for the key name. This ensures hidden characters are displayed, and will prevent super long keys from saturating STDERR with debug output. This means that single quoted keys in error messages are replaced in with double quoted keys with escaped contents. This also includes changes to t/porting/diag.t to detect hv_notallowed() error messages and test them properly.
…d hashes local $hash{key} and \$hash{key} = \$var are both conceptually modify operations which are forbidden when the hash is restricted and the value is readonly. Unfortunately prior to this commit they were still allowed operations. This patch corrects that oversight. Adds a bunch of tests to t/op/lvref.t to ensure that it is illegal to localize or ref-alias a readonly value in a restricted hash. demerphq force-pushed the yves/restricted_hash_disallow_alias_or_local branch from 2f01a2c to 9fa15f1 Compare 
| Is this meant to be fixing #20948? |
Only the part that concerns restricted hashes. |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
This also teaches t/porting/diag.t to handle hv_notallowed() errors. It does not currently include tests, that can come tomorrow.
$ ./perl -Ilib -Mexperimental=refaliasing,declared_refs -le'my %h=(k=>1); Internals::SvREADONLY(%h,1);
Internals::SvREADONLY($h{k},1); my $v2=2; local $h{k} = $v2'
Attempt to alias readonly key "k" in restricted hash at -e line 1.
$ ./perl -Ilib -Mexperimental=refaliasing,declared_refs -le'my %h=(k=>1); Internals::SvREADONLY(%h,1);
Internals::SvREADONLY($h{k},1); my $v2=2; local $h{k} = $v2'
Attempt to localize readonly key "k" in restricted hash at -e line 1.