Modify

#2478 closed Bug (Fixed)

Assign and Eval do not restrict variable names

Reported by: Tasiro Owned by: Jon
Milestone: 3.3.11.1 Component: AutoIt
Version: 3.3.9.21 Severity: None
Keywords: assign, eval, name, restriction Cc:

Description

Local $name = "some space, and a comma"
Assign ($name, 42)
Eval ($name) ; => 42

Is that behavior intended? Should such inaccessable names be allowed?

Attachments (0)

Change History (13)

comment:1 by MHz, on Sep 18, 2013 at 2:43:30 PM

Interesting. I could consider it as a desirable feature.
For instance.

Local $name = "Question, what is 3 + 3?"
Assign($name, 6)
MsgBox(0, '$name', $name & ' ' & @CRLF & Eval($name)  & @CRLF)

You intend to access the string representation of the variable with Eval so I see only benefit. What the name in the variable could be regarded as little concern. You have string functions to check if what is in $name is valid or not by your own criteria. Same as you test for what is put in an edit control of a gui.

comment:2 by anonymous, on Sep 18, 2013 at 2:52:08 PM

Note my testing was done in 3.3.8.1 so this feature has been out in the wild for quite some time.

in reply to:  1 comment:3 by Tasiro, on Sep 18, 2013 at 8:17:48 PM

Replying to MHz:

Interesting. I could consider it as a desirable feature.
For instance.

Local $name = "Question, what is 3 + 3?"
Assign($name, 6)
MsgBox(0, '$name', $name & ' ' & @CRLF & Eval($name)  & @CRLF)

You intend to access the string representation of the variable with Eval so I see only benefit. What the name in the variable could be regarded as little concern. You have string functions to check if what is in $name is valid or not by your own criteria. Same as you test for what is put in an edit control of a gui.

I am afraid I must disagree. There are several disadvantages:

  1. It makes it harder to debug.
  2. It may polute the environment (and makes any good optimization virtually impossible).
  3. The environment is not a map and should not be used as a map.
  4. You cannot access the variable the normal way (no $"Question..." or $$name allowed).

All that glitters is not gold.

I would even deprecate Assign, Eval, IsDeclared, Call and Execute (Call and Execute might have side effects). That kind of reflection should be unneccessary.

comment:4 by MHz, on Sep 19, 2013 at 7:45:27 AM

  1. It is a string. You can test it before using assign so I see this as not an issue.
  2. As above.
  3. So you say but that does not mean that it should not be.
  4. This is why Eval exists. Since when has $"Question..." been allowed? or even $$name? or how about $"var". Perhaps you are thinking of setting the option on for ExpandVarStrings which works fine on "name = $some space, and a comma$" if you wanted to.
  5. Additional point, why submit bugs reports on functions that you do not want to use?

In relation to your mention of gold, gold out of the ground is not good unless it is processed into pure gold. So processing is usually needed.

You can test the string before assigning it for characters you do not want. If you assign a string to become a variable, then you can test it with IsDeclared to test if it exists and if it does, then to what scope it is and you can use Eval to use it. The latter is mentioned in the help file as what you should do on the pages of Assign, IsDeclared and Eval.

Below is code testing the string for being valid for your interest. Added additional _IsDeclared UDF to show Assign, IsDeclared and Eval working together. You can comment the IsAlphaNumUnderScore function to see that it was not allowing the string. You could block assigning the string, convert the string to suit, exit the script or what ever you choose to do. I fail to see how changing the behavior of Assign does any good as you are going to have to check for error and deal with that with more code to correct it. And it is a script breaking change for no good reason that I see at this time.

Tested on 3.3.8.1 and 3.3.9.21. 1 function call tests if the string is just letters, numbers and underscores.

$name = 'some space, and a comma'

If _IsAlphaNumUnderScore($name) Then
	If Assign($name, 46) Then
		MsgBox(0, 'Success', _
			$name & ' = ' & Eval($name) & @CRLF & _
			'Scope = ' & _IsDeclared($name) & @CRLF _
		)
	EndIf
EndIf

Exit

Func _IsAlphaNumUnderScore($string)
	; success if the string is all letters, numbers or underscores
	If StringRegExp($string, '[^a-zA-Z0-9_]') Then Return SetError(1, 0, 0)
	Return 1
EndFunc

Func _IsDeclared($string)
	; IsDeclared but return strings
	Assign('Local: Scope', IsDeclared($string), 1)
	If Eval('Local: Scope') Then
		Switch Eval('Local: Scope')
			Case -1
				Return 'Local'
			Case 1
				Return 'Global'
		EndSwitch
	EndIf
	Return ''
EndFunc

Here is a tip, variables created with Assign, i.e. "var!" will not destroy a variable named $var. You cannot create a variable named $var! so it is safe to assign "var!".

So I do not see an issue, like AutoIt crashing, negative side effects or other. So as per usual with AutoIt, some programming is needed to get the result that you want.

comment:5 by MHz, on Sep 19, 2013 at 8:27:47 AM

Perhaps a request for additional flags?

0 = (default) Create variable if required
1 = Force creation in local scope
2 = Force creation in global scope
4 = Fail if variable does not already exist
8 = Fail if variable does already exist (new)
16 = Fail if not being a string with letters, numbers and underscores (new)

comment:6 by Jon, on Sep 19, 2013 at 8:48:08 AM

Bug as far as I'm concerned.

in reply to:  4 comment:7 by Tasiro, on Sep 19, 2013 at 4:16:30 PM

  1. Bad Idea. "You can't access my variables? Have you tried Eval ("variable name") already?"
    Also, Execute may fail. What will be @extended?
  2. Execute is the (second) slowest way possible.
  3. Use ObjCreate ("Scripting.Dictionary") for maps.
  4. Again, it is a bad idea to use Assign, Eval and Execute. See https://en.wikipedia.org/wiki/Eval.
  5. I cannot deprecate functions of the standard library of AutoIt myself, can I?
    Should I not report bugs I find?
  6. Where is Eval actually neccessary?

I do not think a new flag would be a good idea.

Documentation - Language Reference - Variables:

Each variable has a name (again, similar to a mailbox) and must start with the $ character and may only contain letters, numbers and the underscore _ character.


Anyway, here is my flag proposal:
16 = (not default) Do not fail if varname not being a variable-name (link to specification)

in reply to:  4 comment:8 by Tasiro, on Sep 19, 2013 at 5:14:37 PM

I fail to see how changing the behavior of Assign does any good as you are going to have to check for error and deal with that with more code to correct it.

Then I will have to? Only IsDeclared needs to be changed.

And it is a script breaking change for no good reason that I see at this time.

You cannot expect undefined (or unspecified) behavior to be portable (you seem to be in need of a specification).
Which script would it break?

Here is a tip, variables created with Assign, i.e. "var!" will not destroy a variable named $var. You cannot create a variable named $var! so it is safe to assign "var!".

I may write Assign ("var!", $value), as you do.
Why do you need a variable named "var!"?

So I do not see an issue, like AutoIt crashing, negative side effects or other.

Performance. Consistence. Misuse.

comment:9 by Jpm, on Sep 19, 2013 at 5:16:11 PM

Hold on
Jon acknowledge it is a BUG , I am sure you will be OK with the fix when he will do it

comment:10 by BrewManNH, on Sep 24, 2013 at 6:39:36 PM

One benefit that I've seen, in Assign and IsDeclared taking any string as a valid variable, is some of the code people have used to replace the _ArrayUnique function with something a lot faster. While it's not standard practice to use Assign like that, it has been something people have been posting for several years and in multiple posts. Just something to think about.

comment:11 by czardas, on Oct 12, 2013 at 7:27:31 AM

Ah, you're going to change it.

Although I didn't know about this previously, I think it would be pretty cool if it remained as an undocumented feature. If there are potential issues with behaviour then it should be changed, otherwise I don't see any reason or benefit to change anything. The fact this is even possible seems to open up new possibilities. I made such a function as BrewManNH spoke of, but all variable names use word characters after conversion to hex - allowing case sensitive variable names.

comment:12 by anonymous, on Nov 6, 2013 at 11:32:29 PM

Stripping, restricting or rejecting whitespace/punctuation from the variable name parameters doesn't seem too harsh. Though I don't see the necessity in expending effort to dismantle the entire set of functions.

Is this like a chicken farmer showing his banker how he raises chickens in a sanitary environment, uses only organic feed, how he is producing a bunch of happy, healthy, free-range chickens, and near the end of the tour, they hear calls of "Whoop! Whoop!" and "Yee Haw!" and glance up to see the farm across the road where, in a cloud of dust and feathers, that entire family are out f*cking their chickens.
"I must put a stop to this!", he says?

comment:13 by Jon, on Jan 4, 2014 at 12:00:54 PM

Milestone: 3.3.11.1
Owner: set to Jon
Resolution: Fixed
Status: newclosed

Fixed by revision [9505] in version: 3.3.11.1

Modify Ticket

Action
as closed The owner will remain Jon.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.