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.

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

Fix for #50 #52

Merged
merged 1 commit into from
Jul 23, 2018
Merged

Fix for #50 #52

merged 1 commit into from
Jul 23, 2018

Conversation

das-sein
Copy link
Contributor

This should resolve the issue described by: #50

Cause

Use of AND condition instead of OR condition means that user must be using a free Burp version AND have also not specified a project file.

However, this ignores two cases:

  1. The user mistakenly states a project file with Burp free edition
  2. The user does not state a project file with Burp paid edition

Both of these cases require the creation of an empty projectData array.

Resolution

Changed conditional operator from && to ||.

I also rearranged the operands so that the Note: line flows into the logic of the if-statement.
"Burp Free does not support project..." => "if (!burpEdition.equalsIgnoreCase('free')"

This should resolve the issue described by: #50

Cause: Use of AND condition instead of OR condition means that user must be using a free Burp version AND have also not specified a project file. However, this ignores two cases:

1. The user mistakenly states a project file with Burp free edition
2. The user does not state a project file with Burp paid edition

Both of these cases require the creation of an empty projectData array.

Resolution: Changed conditional operator from `&&` to `||`.

I also rearranged the operands so that the `Note:` line flows into the logic of the if-statement. 
"Burp Free does not support project..." => "`if (!burpEdition.equalsIgnoreCase('free')`"
@vmwclabot
Copy link
Member

@lazorgator, you must sign our contributor license agreement before your changes are merged. Click here to sign the agreement. If you are a VMware employee, read this for further instruction.

@das-sein
Copy link
Contributor Author

I was not able to personally test these changes, so someone definitely should before the merge.

While I was checking out the library to use in a project, I noticed this rather serious bug and fixed it, but am not in a position to test it at the moment.

@vmwclabot
Copy link
Member

@lazorgator, VMware has approved your signed contributor license agreement.

@gangelino gangelino merged commit ac7afc8 into vmware:master Jul 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants