Re: [GRASS-dev] Question regarding GRASS_NOTIFY
I have finally had the time to take a little better look at the code and actually test ximgview/wxpyimgview. I replaced my first PR [1] for deleting GRASS_NOTIFY with a PR [2] with which a G_warning is issued if the system call fails. The documentation on using ximgview/wxpyimgview, including the signal handling, could probably be improved. I will at least file a ticket with that aim. Thank you Glynn for jumping in on this, I very much appreciate it! [1] https://github.com/OSGeo/grass/pull/2135 [2] https://github.com/OSGeo/grass/pull/2705 > On 4 Mar 2022, at 20:28, Glynn Clements wrote: > > > Nicklas Larsson via grass-dev wrote: > >> I personally never had the need for the use of GRASS in this way, so forgive >> my ignorance in this regard. >> However, one thing stands out very clear from my newly gained experience: >> there is a lack of documentation on this use of GRASS_NOTIFY. For instance, >> it is not clear to me is whether it is required or optional to set >> GRASS_NOTIFY >> with e.g.: >> >> export "GRASS_NOTIFY=kill -USR1 `pidof ximgview`” >> >> for this to work as intended. Surely it is not expected a user should look >> for this >> information in the mailing list. > > I think that it's "expected" that the user will just use the GUI. > > It seems doubtful that anyone (other than me) actually used this. I no > longer actively follow GRASS; I only remained subscribed to the list > for cases (like this) where someone is asking about arcane details of > stuff I worked on in the past. > >>> What sort of "sanitisation" would you suggest? The variable is set by >>> the user, its value is passed directly to the shell. >>> >> I’d say it would be better to avoid sanitation, with what I mean making sure >> GRASS_NOTIFY hasn’t been compromised, at all. Couldn’t the desired outcome >> of using GRASS_NOTIFY be implemented in another way? > > Well, it could be made a GRASS ($GISRC) variable. A fixed command name > could be inconvenient if you have multiple GRASS sessions in different > shells (again, I don't think this is "typical" user behaviour). It > could use some other mechanism entirely (e.g. a filename identifying a > socket or named pipe to which D_close_driver would write) would work, > but would be a non-trivial effort for something that probably isn't > even used. > > I'm not sure manipulating GRASS_NOTIFY gets you much compared to > manipulating e.g. EDITOR or BROWSER. And unless there's been a > significant effort on security since I was active, using GRASS with > untrusted inputs or an untrusted environment has much bigger issues. > > In any case, ximgview/wxpyimgview don't appear to have had any > substantial changes or issues (i.e. nothing that doesn't appear to be > a repository-wide clean-up) in the last decade, so non-GUI usage of > the display subsystem is probably a non-issue at this point. > ___ grass-dev mailing list grass-dev@lists.osgeo.org https://lists.osgeo.org/mailman/listinfo/grass-dev
Re: [GRASS-dev] Question regarding GRASS_NOTIFY
On Fri, 4 Mar 2022 at 10:09, Nicklas Larsson via grass-dev < grass-dev@lists.osgeo.org> wrote: > Couldn’t the desired outcome > of using GRASS_NOTIFY be implemented in another way? > The purpose was interactive use from the command line I suppose, so for these cases users can set a convenient alias for sending the signal or a prefix command to use with the display commands (to call the command and then send the signal) like `ni d.rast elevation` where `ni` is the command/alias. Adding something similar to a script has even a smaller overhead, although it too requires you making sure it happens every time instead of the one-time GRASS_NOTIFY settings. (I'm assuming here that signal handling in the imgview programs is not removed.) ___ grass-dev mailing list grass-dev@lists.osgeo.org https://lists.osgeo.org/mailman/listinfo/grass-dev
Re: [GRASS-dev] Question regarding GRASS_NOTIFY
Nicklas Larsson via grass-dev wrote: > I personally never had the need for the use of GRASS in this way, so forgive > my ignorance in this regard. > However, one thing stands out very clear from my newly gained experience: > there is a lack of documentation on this use of GRASS_NOTIFY. For instance, > it is not clear to me is whether it is required or optional to set > GRASS_NOTIFY > with e.g.: > > export "GRASS_NOTIFY=kill -USR1 `pidof ximgview`” > > for this to work as intended. Surely it is not expected a user should look > for this > information in the mailing list. I think that it's "expected" that the user will just use the GUI. It seems doubtful that anyone (other than me) actually used this. I no longer actively follow GRASS; I only remained subscribed to the list for cases (like this) where someone is asking about arcane details of stuff I worked on in the past. > > What sort of "sanitisation" would you suggest? The variable is set by > > the user, its value is passed directly to the shell. > > > I’d say it would be better to avoid sanitation, with what I mean making sure > GRASS_NOTIFY hasn’t been compromised, at all. Couldn’t the desired outcome > of using GRASS_NOTIFY be implemented in another way? Well, it could be made a GRASS ($GISRC) variable. A fixed command name could be inconvenient if you have multiple GRASS sessions in different shells (again, I don't think this is "typical" user behaviour). It could use some other mechanism entirely (e.g. a filename identifying a socket or named pipe to which D_close_driver would write) would work, but would be a non-trivial effort for something that probably isn't even used. I'm not sure manipulating GRASS_NOTIFY gets you much compared to manipulating e.g. EDITOR or BROWSER. And unless there's been a significant effort on security since I was active, using GRASS with untrusted inputs or an untrusted environment has much bigger issues. In any case, ximgview/wxpyimgview don't appear to have had any substantial changes or issues (i.e. nothing that doesn't appear to be a repository-wide clean-up) in the last decade, so non-GUI usage of the display subsystem is probably a non-issue at this point. ___ grass-dev mailing list grass-dev@lists.osgeo.org https://lists.osgeo.org/mailman/listinfo/grass-dev
Re: [GRASS-dev] Question regarding GRASS_NOTIFY
> On 3 Mar 2022, at 22:20, Glynn Clements wrote: > > > Nicklas Larsson via grass-dev wrote: > >> Is there anyone out there who know about usage of the environment >> variable GRASS_NOTIFY, historically and in the present? > > It's an external interface; it isn't used internally. > > The intention is described in the email you cited: to allow a program > (specifically, ximgview or similar) to be notified of any changes to > the image file generated by the display library. It isn't required by > the GUI because the GUI can assume that the image will only be > modified by d.* commands which the GUI itself runs, so it can refresh > the view upon their completion. > > The idea was to support the use of d.* commands from a shell, > providing something similar to the "monitor" system that was being > replaced at the time. I've have no idea whether such usage is still > common or even officially supported (but scripts/wxpyimgview is still > there). > I personally never had the need for the use of GRASS in this way, so forgive my ignorance in this regard. However, one thing stands out very clear from my newly gained experience: there is a lack of documentation on this use of GRASS_NOTIFY. For instance, it is not clear to me is whether it is required or optional to set GRASS_NOTIFY with e.g.: export "GRASS_NOTIFY=kill -USR1 `pidof ximgview`” for this to work as intended. Surely it is not expected a user should look for this information in the mailing list. >> In addressing an otherwise trivial compiler warning, I stumbled into >> this for me strange piece of code and usage of GRASS_NOTIFY in the >> Raster Display Library. I put up a PR [1] suggesting the removal of >> this, but I agree with Vaclav I’d better ask here on the ML. >> >> The only use of GRASS_NOTIFY is in “D_close_driver()” [2] and the >> value of that env variable (if set) is directly inserted in a >> “system()” call, without any sanitation! > > What sort of "sanitisation" would you suggest? The variable is set by > the user, its value is passed directly to the shell. > I’d say it would be better to avoid sanitation, with what I mean making sure GRASS_NOTIFY hasn’t been compromised, at all. Couldn’t the desired outcome of using GRASS_NOTIFY be implemented in another way? (It could, probably rightfully, be argued that this is not the single one weakness in GRASS code and in addition a very unlikely attack vector, but why leave doors open if not needed). ___ grass-dev mailing list grass-dev@lists.osgeo.org https://lists.osgeo.org/mailman/listinfo/grass-dev
Re: [GRASS-dev] Question regarding GRASS_NOTIFY
Nicklas Larsson via grass-dev wrote: > Is there anyone out there who know about usage of the environment > variable GRASS_NOTIFY, historically and in the present? It's an external interface; it isn't used internally. The intention is described in the email you cited: to allow a program (specifically, ximgview or similar) to be notified of any changes to the image file generated by the display library. It isn't required by the GUI because the GUI can assume that the image will only be modified by d.* commands which the GUI itself runs, so it can refresh the view upon their completion. The idea was to support the use of d.* commands from a shell, providing something similar to the "monitor" system that was being replaced at the time. I've have no idea whether such usage is still common or even officially supported (but scripts/wxpyimgview is still there). > In addressing an otherwise trivial compiler warning, I stumbled into > this for me strange piece of code and usage of GRASS_NOTIFY in the > Raster Display Library. I put up a PR [1] suggesting the removal of > this, but I agree with Vaclav I’d better ask here on the ML. > > The only use of GRASS_NOTIFY is in “D_close_driver()” [2] and the > value of that env variable (if set) is directly inserted in a > “system()” call, without any sanitation! What sort of "sanitisation" would you suggest? The variable is set by the user, its value is passed directly to the shell. ___ grass-dev mailing list grass-dev@lists.osgeo.org https://lists.osgeo.org/mailman/listinfo/grass-dev
