mIRC Home    About    Download    Register    News    Help

Print Thread
#271627 05/05/23 02:41 PM
Joined: Jan 2004
Posts: 2,127
maroon Offline OP
Hoopy frood
OP Offline
Hoopy frood
Joined: Jan 2004
Posts: 2,127
Suggesting tighter rules for accepting $urlget targets for binvars and filenames.

  • Block &binvar name containing spaces

    https://forums.mirc.com/ubbthreads.php/topics/266018/re-binvar-named#Post266018

    I'm only suggesting consistency between commands and identifiers, and I'm perfectly fine with NOTHING accepting a space inside a &binvarname.

    Related to the earlier thread about consistent validation of &binvar names, $urlget permits the name of a &binvar to contain a space and then triggers the callback alias with .state being 'ok'. While $bvar() and $bfind can handle that, /bset /bcopy /bwrite cannot, even if using $qt(target). I found this while trying to figure out a way to pass a commandline to the callback alias, and picked the wrong parm. When I made the 'target' parm be '&binvar text', $urlget created a &binvarname containing a space, and $bvar($urlget($1).target,0) $bvar($urlget($1).target,1-).text etc work like normal, but /bwrite /bset /bcopy failed.

    So $urlget could probably either reject this as an invalid binvarname the same way it does if the parm doesn't begin with '&' - or else call the callback alias with $urlget(id).state being 'fail' like it does if the f=target filename is global.txt
    .
  • possible new $urlgetdir identifier

    I'm wondering how useful it would be to 'sandbox' the $urlget target=f downloads by having $urlgetdir default to match $getdir for backwards compatibility, but being able to configure $urlgetdir to point elsewhere.

    For most identifiers and commands, the absence of a target path means the target points to $mircdir. The exception is for DCC downloads where the file downloads to $getdir(filename) which are able to be defined as pointing to $wavedir etc.

    However $urlget is a hybrid, where target-using-no-path means writing to $getdir even if $getdir(target-filename.filetype) points to somewhere else like c:\sounds\ or $mircdir $+ sounds. But if you use a relative path, then the base folder changes from $getdir to $mircdir.

    For example, if $getdir is H:\DOWNLOADS\ and $mircdir is C:\MIRC\ then f=target being test.txt downloads as H:\DOWNLOADS\test.txt but if the target is .\test.txt then it downloads as C:\MIRC\test.txt
    .
  • Block .. and possibly also . in f=target?

    I'm wondering if it's a good idea for $urlget to either reject paths containing .. folders, or to have something like a 'F' switch which either ignores the path in a target or makes the target invalid if it contains \ or /. If someone is using $urlget to download files from a file list generated somewhere else such as located inside a url retrieved from $urlget etc - without sanitizing them - there could be shenanigans like having the list salted with .\mirc.ini or .\scripts\file.mrc downloads\..\scripts\file.mrc etc.

    While there may be legit cases where a target containing a .. path folder would be legit, it may be a good idea for $urlget to not accept ..\ in paths. While the /fserve sandboxes downloads from escaping the homedir, /dcc send allows sending a path containing ..\

    If someone is using a downloads file list created by someone else, then even if $getdir is sandboxed to point to H:\downloads\, and $urlget would download the target parameter testfoo.dll as H:\downloads\testfoo.dll, the f=target parameter as ..\windows\system32\testfoo.dll would download to the c: drive if $mircdir is on the c: drive and if $isadmin is $true and if the quantity of ..\ matches the depth of $mircdir below the root.

Joined: Jul 2006
Posts: 4,149
W
Hoopy frood
Offline
Hoopy frood
W
Joined: Jul 2006
Posts: 4,149
Hey

1) is just a bug, allowing binvar name with space in them means no manipulation possible with commands as you said, and that basically defeats the purpose of having such binvar name. it should report an error immediately
1.1) related topic is loading item name with space with /hload by editing an item name in a file manually, $readini $ini etc are also affected, it would be great to be able to have better control over this, /hload could get a new switch to a) return an error when an item with space is found b) accept the item name with space (default) c) accept the item but convert all spaces to underscore or something.
$readini & $ini could get a new switch ($ini doesn't have switch but there recent $ini suggestion taking some) to a) prevent a match if the item/section name contains spaces (for both text comparision and if the Nth item/section refers to an item/section with a space, for $ini)

2) $urlgetdir wouldn't solve anything, it wouldn't be sandboxing much as that value could then points to $mircdir anyway, I don't think dcc related settings should be applied either to get the dir.
It's all command and identifiers which starts at $mircdir except for $urlget, $getdir isn't even an exception because it's doesn't take a parameter used by mirc to read or write to/from it, it's mainly using the parameter for string comparison without checking the existence of the file.
2.1) As discussed on IRC $urlget, with a file target parameter value of "subfolder\filename.txt" $urlget does NOT make the complete filename starts at $getdir but at $mircdir, whereas using a file target parameter of "filename.txt" does make the complete filename starts at $getdir. Both are easily called "relative path" but they don't end up using the same path, this I consider a bug if $urlget is always meant to make relative path starts at $getdir instead of the usual $mircdir. I wrote a post in the past about how I believe it should be made consistent with other identifiers and always use $mircdir for relative path but between it being the only identifier doing that and the fact that "subfolder\filename" starts at $mircdir anyway, which is inconsistent for $urlget itself, I think backward compatibility are compromised for $urlget anyway and that it should be changed. Think of it like the on keydown event, that was also trying for years to combine both keypress and resulting characters, resulting in new on char event added, which broke compatibility for the on keydown event but we ended up with a much better feature.

3) I think that .. and . should be handled the same in all identifiers and command's parameter which are used by mirc to read and write to/from file, but that would break compatibility in a bad way. There's already a lot of different behavior, $sfile won't work with .. but $findfile will. I believe most commands and identifiers decodes .. properly though, maybe only a couple don't, so I'd say $urlget should decode them correctly, there's many way to screw up with a powerful language like msl, limiting functionalities because of the argument 'it's dangerous' makes no sense given what you can already do.


#mircscripting @ irc.swiftirc.net == the best mIRC help channel
Joined: Jan 2004
Posts: 2,127
maroon Offline OP
Hoopy frood
OP Offline
Hoopy frood
Joined: Jan 2004
Posts: 2,127
Re Wims:

1.0

True, there's no real usefulness in having a &binvarname containing a space. Directly related to fixing the $urlget parsing of the &binvar target would likely fix 2 other ways to create &binvarname containing spaces, or where %var and its contents can be interpreted in 3 ways:

1a. $hget creates binvar named "&v text"

//hfree -w foo | hadd -m1 foo item data | echo 4 -a $hget(foo,item,&v text) : $bvar(&v text) vs $bvar(&v) vs $bvar(&v text,0)

1b. $regsubex (and $regsub) creates binvar named "&v text"

//echo -a noop $regsub(foo,test,,,&v text) = 1 = success | echo 4 -a $hget(foo,item,&v text) : exists: $bvar(&v text) not exist: $bvar(&v) size 4: $bvar(&v text,0)

1c. $regsubex and $regsub create %zzyzyx where $var() thinks the varname is %zzyzyx containing 'test data', but /echo thinks that %zzyzyx does not exist, and $len() and $left() think the varname is '%zzyzyx test' containing 'data'

//unset %zzyzyx* | echo -a noop $regsubex(foo,data,,,%zzyzyx test ) | echo 4 -a varname $var(%zzyzyx,1) value $var(%zzyzyx,1).value : name.len $len($var(%zzyzyx,1)) datalen $len($var(%zzyzyx,1).value) :: *$len of content: $len(%zzyzyx test) vs $len(%zzyzyx) content $left(%zzyzyx test,99) vs null: %zzyzyx

1.1

The related issue for /hload was suggested here:

https://forums.mirc.com/ubbthreads.php/topics/266694/hashtable-item-names-with-spaces

The most common way that hload would be loading itemnames containing spaces would be an error due to not loading in the exact same format used by /hsave, so it might be best to change /hload to have the default behavior of considering an itemname containing a space to be invalid except if using a brand new switchletter.

However, it can be useful to have spaces in a hashtable itemname, such as having filenames be the filename without being forced to use $encode(filename,m) as the itemname, where you're forced to choose between either having filename-strings being case-sensitive or losing the upper/lower by using $encode($upper(filename),m) as the itemname.

Changing spaces to underscores in itemnames is something I'm not so sure i'd be in favor of, since it would create the scenario where itemnames "foo_bar" and "foo bar" would have the contents of whichever is the last one loaded. Hopefully it wouldn't borrow the behavior from DCC where ^ in filenames are also changed to _'s.

The other way to handle hashtable spaces is like I suggested in the other link, with a -q switch indicating to /hadd /hdel /hinc /hdec that an itemname beginning with " should indicate this is a quoted string for the filename.

2.0

My suggestion of $urlget was trying to find a way to fix things in a backwards compatible way, where $urlget being different than $getdir would the more secure behavior of having the non-relative no-path-filenames and /relative/filename both being 'related' to the same folder location. A new switch could accomplish the same. By sandboxed, I meant that relative paths without a drive letter wouldn't allow escaping the H: drive so the ..\ shenanigans wouldn't work.

$urlget was also trying to get there to be a way that urlget behaves more like the other /write commands where no-path means the target is in $mircdir. For $urlget I had always been fetching to target &binvars instead of to disk files, so when trying to fix the GPF problem involving /hsave -i, Khaled had created a testing script that was downloading to unique filenames. Since the target contained no relative-path, I thought the file would be downloading to $mircdir, and I was confused because no files were showing up. By the time I figured out that the files were going into $getdir instead, there were over 100,000 files in the downloads folder, and it was a big mess trying to deal with that. So, a $urlget and/or switch would let users decide whether they wanted to point $urlgetdir to $mircdir, or to $getdir, or to create a urlgetfolder/ for them.

Joined: Jan 2004
Posts: 2,127
maroon Offline OP
Hoopy frood
OP Offline
Hoopy frood
Joined: Jan 2004
Posts: 2,127
Update: 2 additional identifiers that allow creating &binvar names containing a space.

1d. $fread

//fopen foo mirc.ini | echo -a $fread(foo,10,&foo bar) | echo -a $bvar(&foo) vs $bvar(&foo bar) | fclose foo

1e. $cb

//clipboard test | echo -a $cb(-1,,&foo bar) : $bvar(&foo) vs $bvar(&foo bar)


Link Copied to Clipboard