-
-
Notifications
You must be signed in to change notification settings - Fork 87
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
First push of new independent Python API for ZAP #1
base: main
Are you sure you want to change the base?
Conversation
target = 'http://127.0.0.1' | ||
|
||
# By default ZAP API client will connect to port 8080 | ||
zap = ZAPv2(proxies={'http': '127.0.0.1:9090', 'https': '127.0.0.1:9090'}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should specify the proxy in this statement? If so, the comment and following example needs to be updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a mistake. I'll fix it ni next PR with new example
fix: removed redundant license file
required = f.read().splitlines() | ||
|
||
setup( | ||
name="python-owasp-zap-v2.4", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hum, should this be renamed to python-owasp-zap-v2.5 ? As 2.5.0 is coming out soon?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really, IMHO, the better choice si to use a standard name and unify the version. So there's a "version" param in setup(..). Currently in Pypi are available 3 different libraries:
This is not a the correct way to send an update to Pypi. I'll change the name to:
setup(name="python-owasp-zap" ...
And only update the "version" param:
setup(... version=1.0.0 ...
For Python developers is more intuitive and easy if only one library is available, with different versions release.
When we update for a new version, not change the name name of library, only de version param, doing:
python setup.py sdist upload
Of, for the first release (if we change the name):
python setup.py sdist register upload
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you agree, I'll send a PR with the change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Python library is using those names to prevent "incompatibility issues" between releases of ZAP. For example, newer versions include functionalities that will not work with older versions of ZAP, so the v2.4 (or v2.5) indicates the version of ZAP that the client implementation is targeting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To do that, I think a better approach with having only one library, but with a different API. This is:
For version 2.4 of ZAP
from zap import ZAP_24
c = ZAP_24()
For version 2.5 of ZAP
from zap import ZAP_25
c = ZAP_25()
And so on. This way, all ZAP versions are unified in only one library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The API is continually evolving, and will carry on doing so for the foreseeable future.
And we know users arent able to update to new versions of ZAP immediately, especially if any refactoring is involved.
I think we do need a plan for this, and I also like the idea of having just one library.
So a proposal would be great :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea could be:
New organization of code, refactoring the Python entry point classes (ZAP_23, ZAP_25..) but, the entry points generated automatically with Java. Doing this, maybe we can unify the two approaches.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oka, I'll send a PR proposal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just sent the proposal :)
Done with my comments - cant comment on the python code but looks better than the stuff I hacked together ;) |
Hi @psiinon! What's about pull requests? |
Ah, I was assuming we would need to sort out the proxying problem before the new code could be merged. Am I wrong? |
What's the proxying problem? I don't know if I understand what you mean :) |
Just going to write my thoughts about this PR. First things first, I don't even know if this works with Python3. I see that you have imported the API accessor ( |
No description provided.