carlos-montiers / enhancedbatch

Enhances your windows command prompt https://www.enhancedbatch.com
Other
5 stars 1 forks source link

Patch FOR /F ... IN('command') ... to disable autorun #33

Open DaveBenham opened 4 years ago

DaveBenham commented 4 years ago

Pipes are implemented by executing each side in a new cmd process with the /D option to disable autorun, which is a good thing.

But the developer of FOR /F forgot to add the /D option when executing the IN('command'). Could you patch FOR /F to fix this oversight? If need be you could add a new option "noautorun" to optionally disable autorun, thus preserving existing behavior by default.

adoxa commented 4 years ago

We discussed this in email. FOR just calls popen, so it's not a simple patch. My suggestion was to simply run cmd /d /c ... explicitly, recognising it's not really ideal.

DaveBenham commented 4 years ago

My suggestion was to simply run cmd /d /c ... explicitly, recognising it's not really ideal.

Ouch. I don't think that solves anything. The implicit cmd /c would still execute the autorun before the explicit cmd /d /c.

That is unfortunate.

adoxa commented 4 years ago

It's still possible to replace popen with a custom version, just more work that I don't really want to do.

carlos-montiers commented 4 years ago

I think if we care about not run autorun commands we also will need provide an alternative START command because it uses cmd /c (without /d) @adoxa If I provide the custom popen function, is complicated patch for /f for use it? I think that with for /f we always return to the question about how much patch it instead of write a new for /f replacement, that is not the goal. But if is easy plug for /f with a custom popen, I can provide it.

carlos-montiers commented 4 years ago

The custom popen I think is needed provide for solve the issue #12 and allow implement SPAWN and PIPE

adoxa commented 4 years ago

START uses cmd /k, creating a new window, so autorun should probably not be disabled.

Popen is simple enough to do, it's just having to do it (grumble, grumble).

carlos-montiers commented 4 years ago

@adoxa, Now that we have My_popen and My_wpopen, that is not a full implementation of popen, currently is a wrapper. Maybe is too complex implement popen from scratch?. Maybe cancel the idea of full implement popen, only for the idea of disable the autorun. Also, I have doubts about the idea of implement it, because maybe each windows version have different implementations, and maybe in the next windows version our popen implementation can not work properly? What do you think?

adoxa commented 4 years ago

I think you keep mentioning it so I'll have to do it...eventually. There should be no problem implementing a custom popen.