Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix /" #1303

Merged
merged 3 commits into from
Oct 16, 2016
Merged

Fix /" #1303

merged 3 commits into from
Oct 16, 2016

Conversation

Simon311
Copy link
Contributor

Fixes #1301.

hakusaro
hakusaro requested changes Oct 16, 2016
View reviewed changes
Copy link
Member

@hakusaro hakusaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested in game and confirmed that the exploit is no longer possible. Functionally looks good. Just minor style changes then I'm happy.

TShockAPI/Commands.cs
return false;
int index = -1;
for (int i = 0; i < cmdText.Length; i++)
if (IsWhiteSpace(cmdText[i]))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whether or not this is syntactically correct or not, stylistically there should be some brackets around this for loop.

hakusaro
hakusaro approved these changes Oct 16, 2016
View reviewed changes
Copy link
Member

@hakusaro hakusaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested in game all working commands too.

lgtm

@hakusaro
Copy link
Member

Let me clarify that last comment: I tested and made sure that command parsing wasn't changed too.

@Simon311
Copy link
Contributor Author

Let me fix something quickly

@Simon311
Copy link
Contributor Author

Once this is accepted, I will push commits and make another PR fixing #1206.

@AxisKriel AxisKriel merged commit 0ec49ab into Pryaxis:general-devel Oct 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hakusaro hakusaro hakusaro approved these changes

@AxisKriel AxisKriel AxisKriel approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Simon311 @hakusaro @AxisKriel