First push of new independent Python API for ZAP by cr0hn · Pull Request #1 · zaproxy/zap-api-python

cr0hn · 2016-04-20T16:14:07Z

No description provided.

thc202 · 2016-04-20T16:20:01Z

examples/usage_example.py

+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'})


Should specify the proxy in this statement? If so, the comment and following example needs to be updated.

It's a mistake. I'll fix it ni next PR with new example

fix: removed redundant license file

psiinon · 2016-04-22T08:47:58Z

setup.py

+    required = f.read().splitlines()
+
+setup(
+    name="python-owasp-zap-v2.4",


Hum, should this be renamed to python-owasp-zap-v2.5 ? As 2.5.0 is coming out soon?

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

If you agree, I'll send a PR with the change

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.

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.

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 :)

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.

Works for me :)

Oka, I'll send a PR proposal

I just sent the proposal :)

psiinon · 2016-04-22T08:57:54Z

Done with my comments - cant comment on the python code but looks better than the stuff I hacked together ;)

cr0hn · 2016-06-02T13:04:19Z

Hi @psiinon! What's about pull requests?

psiinon · 2016-06-02T13:40:27Z

Ah, I was assuming we would need to sort out the proxying problem before the new code could be merged. Am I wrong?
I've submitted #3 for updating the code in the 'old' way, but v happy for that to be ditched once the new code is working fine...

cr0hn · 2016-06-07T09:46:38Z

What's the proxying problem? I don't know if I understand what you mean :)

Woolworths · 2016-06-10T06:12:48Z

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 (from .acsrf import acsrf) but you fail to include them in the project. At the moment, the API accessors (spider, ascrf, ascan ...) do not support Python3 (as they use {}.iteritems() which is removed in Python3).
Next you do this (in _request_other):
get=None if get=None: get={}
Why?
You also do this: "%s?%s" % (url, urlencode(get)) which makes no sense as you are using requests (where you can just do params=get).
Furthermore you do this: return json.loads when again, requests has it's own builtin function (.json()) which you can't use because you prematurely append .text in your request.
In status_code() you open up a new requests.get instead of self.urlopen, meaning that there are now two requests you need to take care of. This is evident as you use proxies=self.__proxies twice.
You also have the same class twice (base.py and zap_24/__init__.py). This just over complicates the project, for literally no reason.
Also I think that we should be moving away from using project numbers in class names. Not only do they provide no advantage, but make maintaining harder.

First push of new independent Python API for ZAP

28f9d6c

thc202 reviewed Apr 20, 2016
View reviewed changes

fix: Examples

0b9da30

fix: removed redundant license file

psiinon reviewed Apr 22, 2016
View reviewed changes

New refactor of code

8368597

thc202 mentioned this pull request Jun 9, 2016

Fix for Python 3 #4

Merged

Uh oh!

Conversation

cr0hn commented Apr 20, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

psiinon commented Apr 22, 2016

Uh oh!

cr0hn commented Jun 2, 2016

Uh oh!

psiinon commented Jun 2, 2016

Uh oh!

cr0hn commented Jun 7, 2016

Uh oh!

Woolworths commented Jun 10, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

Woolworths commented Jun 10, 2016 •

edited

Loading