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 zaproxy/zaproxy #1405 - Field Enumeration #951

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@AnamikaD
Contributor

AnamikaD commented Jul 11, 2017

Addon for Field Enumeration

@thc202 thc202 referenced this pull request Jul 11, 2017

Closed

Field Enumeration #3743

@kingthorin

This is going to seem like a lot of things to address but they're all really simple. You can probably knock them all off in 10-20m. [Feedback is just based on reading, I haven't built it and used it yet.]

There seems to be two description strings used Provides blacklist characters and Identify blacklist characters. I don't want to be too picky but thinking as a user I'd have no idea what either means or is intended to imply.

Show outdated Hide outdated build/build.xml
Show outdated Hide outdated build/build.xml
Show outdated Hide outdated build/build.xml
@@ -0,0 +1,87 @@
package org.zaproxy.zap.extension.fieldenumeration;

This comment has been minimized.

@kingthorin

kingthorin Jul 11, 2017

Member

Missing standard license header.

@kingthorin

kingthorin Jul 11, 2017

Member

Missing standard license header.

This comment has been minimized.

@AnamikaD

AnamikaD Aug 11, 2017

Contributor

done

@AnamikaD

AnamikaD Aug 11, 2017

Contributor

done

Show outdated Hide outdated ...rg/zaproxy/zap/extension/fieldenumeration/ExtensionFieldEnumeration.java
@@ -0,0 +1,43 @@
package org.zaproxy.zap.extension.fieldenumeration;

This comment has been minimized.

@kingthorin

kingthorin Jul 11, 2017

Member

Should include standard license header.

@kingthorin

kingthorin Jul 11, 2017

Member

Should include standard license header.

This comment has been minimized.

@AnamikaD

AnamikaD Aug 11, 2017

Contributor

done

@AnamikaD

AnamikaD Aug 11, 2017

Contributor

done

Show outdated Hide outdated src/org/zaproxy/zap/extension/fieldenumeration/PopupMenuField.java
Show outdated Hide outdated ...rg/zaproxy/zap/extension/fieldenumeration/ExtensionFieldEnumeration.java
} catch (MalformedURLException e) {
return null;
}
}

This comment has been minimized.

@kingthorin

kingthorin Jul 11, 2017

Member

Should include an overridden getUIName() method that returns an user friendly i18n name for the extension (as displayed in the Options > Extensions panel)

@kingthorin

kingthorin Jul 11, 2017

Member

Should include an overridden getUIName() method that returns an user friendly i18n name for the extension (as displayed in the Options > Extensions panel)

This comment has been minimized.

@AnamikaD

AnamikaD Aug 11, 2017

Contributor

done

@AnamikaD

AnamikaD Aug 11, 2017

Contributor

done

Show outdated Hide outdated src/org/zaproxy/zap/extension/fieldenumeration/ZapAddOn.xml
@thc202

This comment has been minimized.

Show comment
Hide comment
@thc202

thc202 Jul 11, 2017

Member

@kingthorin note that this was not yet intended for review, it was just to keep us updated with the latest changes.

Member

thc202 commented Jul 11, 2017

@kingthorin note that this was not yet intended for review, it was just to keep us updated with the latest changes.

@kingthorin

This comment has been minimized.

Show comment
Hide comment
@kingthorin

kingthorin Jul 11, 2017

Member

Oh, sorry. Jumped the gun ;)

Member

kingthorin commented Jul 11, 2017

Oh, sorry. Jumped the gun ;)

@AnamikaD

This comment has been minimized.

Show comment
Hide comment
@AnamikaD

AnamikaD Sep 4, 2017

Contributor

@kingthorin @thc202 @psiinon Please review this!

Contributor

AnamikaD commented Sep 4, 2017

@kingthorin @thc202 @psiinon Please review this!

@thc202

This comment has been minimized.

Show comment
Hide comment
@thc202

thc202 Sep 4, 2017

Member

Doing that, will add comments shortly.

Member

thc202 commented Sep 4, 2017

Doing that, will add comments shortly.

private static final long serialVersionUID = 1L;
private static final Logger logger = Logger.getLogger(PopupMenuField.class);

This comment has been minimized.

@kingthorin

kingthorin Sep 5, 2017

Member

private static final Logger LOGGER= Logger.getLogger(PopupMenuField.class);

Full caps for constants.

@kingthorin

kingthorin Sep 5, 2017

Member

private static final Logger LOGGER= Logger.getLogger(PopupMenuField.class);

Full caps for constants.

@AnamikaD

This comment has been minimized.

Show comment
Hide comment
@AnamikaD

AnamikaD Sep 5, 2017

Contributor

@thc202 @kingthorin Thank you for the review. Except for "abstract frames" and code modification in terms of logic, all are done. I have clicked on thumbs up icon to indicate completed tasks

Contributor

AnamikaD commented Sep 5, 2017

@thc202 @kingthorin Thank you for the review. Except for "abstract frames" and code modification in terms of logic, all are done. I have clicked on thumbs up icon to indicate completed tasks

@AnamikaD

This comment has been minimized.

Show comment
Hide comment
@AnamikaD

AnamikaD Sep 12, 2017

Contributor

@thc202 @kingthorin @psiinon Please review the final changes!

Contributor

AnamikaD commented Sep 12, 2017

@thc202 @kingthorin @psiinon Please review the final changes!

Addons Field Enumeration
Initial contribution.
@kingthorin

Squashed, added commons-csv-1.1.jar to .classpath

Dismissing review - Outdated

@kingthorin kingthorin changed the title from Fix zaproxy/zaproxy #1405 to Fix zaproxy/zaproxy #1405 - Field Enumeration Jan 8, 2018

@kingthorin

This comment has been minimized.

Show comment
Hide comment
@kingthorin

kingthorin Mar 13, 2018

Member

As discussed via IRC this could also benefit from a thread pool.

Member

kingthorin commented Mar 13, 2018

As discussed via IRC this could also benefit from a thread pool.

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