Skip to content
This repository was archived by the owner on Mar 17, 2024. It is now read-only.

Added the ability to upload an execute a batch file.#312

Merged
MaxXor merged 3 commits intoquasar:masterfrom
tidusjar:master
Jul 31, 2015
Merged

Added the ability to upload an execute a batch file.#312
MaxXor merged 3 commits intoquasar:masterfrom
tidusjar:master

Conversation

@tidusjar
Copy link
Copy Markdown
Contributor

Feature for #244

@werkamsus
Copy link
Copy Markdown

Does hidden execution work on them too? If not you might want to add that c:

@yankejustin
Copy link
Copy Markdown
Contributor

This PR, from the title, seems to only be for support of uploading it, not executing it.
I'm actually not quite sure if we can execute executable files hidden already.

@yankejustin
Copy link
Copy Markdown
Contributor

As a note to clarify my comment above, I would just like to say that nothing really has to be done about the execution of the file. It should already be functioning as well as the execution feature of a normal batch file.

@tidusjar
Copy link
Copy Markdown
Contributor Author

@werkamsus yes it already had the functionality for hidden files. I just built on top of that.

@yankejustin good suggestions. Could you expand on point 2 & 3? I had a look how uploading and executing an exe worked and it's just using simply starting a process. This will also work with batch files and the original request is to upload and run .bat files. This achieves that. I could have wrote a lot more code and changed things but I decided against it since what was there works, so I just expanded upon it.

@yankejustin
Copy link
Copy Markdown
Contributor

@tidusjar Are you asking about my 2nd and 3rd points from the first comment or the 2nd and 3rd comments? :)
The second and third comments were responses to werkamsus. I was just stating that the existing code for starting the process works pretty good and that this pull request does exactly how it is titled.
In the original comment, the comment I made is actually linked to the line of code I was referring to. It is just a suggestion to improve the code before it is merged with the base repository. 💩

@tidusjar
Copy link
Copy Markdown
Contributor Author

Ah ok that makes sense!
Regarding
2. Perhaps we should make sure there are two objects in the Block array before indexing the array for positions 0 and 1.
3. Why check command.CurrentBlock == 0 twice?

On your second point do you mean check that there are atleast [0] and [1] before checking, then no exceptions are thrown? Good idea.

  1. It's basically two condition checks, in both conditions it needs to be 0. I could move that check above and then short circuit it to save some time but I didn't think it was worth it. But I can change that now.

@MaxXor MaxXor merged commit 4d0ae7b into quasar:master Jul 31, 2015
@yankejustin
Copy link
Copy Markdown
Contributor

Sometimes it is dangerous to assume we can call comand.Block[0] because we can not always be certain the client sent a packet that was correctly-formed. We should check if (command.Block.Count >= 2 before indexing it so we know we can't get an IndexOutOfRangeException.
Also, the condition actually goes like this:

  1. Check if the CurrentBlock equals 0.
  2. Index the array (Block) at position 0 and position 1.
  3. Check if the CurrentBlock equals 0.
  4. Index the array (Block) at position 0 and position 1.

I was suggesting removing the unnecessary 3rd part as mentioned right above. It has no need to check the CurrentBlock twice.

@yankejustin
Copy link
Copy Markdown
Contributor

Perhaps you meant to include || in the conditional.

@MaxXor
Copy link
Copy Markdown
Contributor

MaxXor commented Jul 31, 2015

Anyways guys, I've fixed the changes and merged it. #313

@tidusjar
Copy link
Copy Markdown
Contributor Author

Sorry I made changes before i seen these. Thanks for the comments.

@yankejustin
Copy link
Copy Markdown
Contributor

Good. :)
Thank you for the contribution.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants