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

spiderAjax: allow choose context/user and set URL #468

Merged
merged 1 commit into from
Aug 30, 2016

Conversation

thc202
Copy link
Member

@thc202 thc202 commented Jul 21, 2016

Change AJAX Spider dialogue to allow to specify the context and the user
and manually set the start URL.
Change the AJAX Spider API to allow to specify the context and the user
(it was already possible to specify the start URL).
Update changes in ZapAddOn.xml file.
Fork Crawljax's Crawler class (tag 3.6) to allow to spider all URLs
(e.g. a context with multiple hosts included).

Fix zaproxy/zaproxy#1955 - Allow to AJAX spider a context
Fix zaproxy/zaproxy#1956 - Allow to AJAX spider as a user
Fix zaproxy/zaproxy#1957 - Allow URL to be pasted into AJAX Spider
dialogue

@psiinon
Copy link
Member

psiinon commented Jul 25, 2016

I'm nervous about forking 3rd party classes.
Have you tried sending a PR to the Crawljax project, if so have they responded?
If forking is the only option how well are the changes documented? I'm worried they will be lost if we update to a newer version of Crawljax.

@thc202
Copy link
Member Author

thc202 commented Jul 25, 2016

I didn't try, (public?) development of Crawljax is pretty stale (last commit/merge was 8 months ago).
The changes have the usual ZAP comment (I can improve that if it's not enough).

@psiinon
Copy link
Member

psiinon commented Aug 24, 2016

@thc202 did you raise a Crawljax issue? I agree its unlikely to be acted upon, but at least it shows we tried ;)

@thc202
Copy link
Member Author

thc202 commented Aug 24, 2016

Not yet, will check if there's one raised already.

@thc202
Copy link
Member Author

thc202 commented Aug 24, 2016

The closest issue I could find was crawljax/crawljax#93, the issue would allow to crawl other sites (based on the comments posted in the pull requests to fix the issue).
Worth adding a comment or raise a new one?

@psiinon
Copy link
Member

psiinon commented Aug 24, 2016

Either is fine by me :)

@thc202
Copy link
Member Author

thc202 commented Aug 24, 2016

Comment added to the mentioned issue.

Is there any change that I need to do to the pull request (other than rebase)?

Change AJAX Spider dialogue to allow to specify the context and the user
and manually set the start URL.
Change the AJAX Spider API to allow to specify the context and the user
(it was already possible to specify the start URL).
Update changes in ZapAddOn.xml file.
Fork Crawljax's Crawler class (tag 3.6) to allow to spider all URLs
(e.g. a context with multiple hosts included).

Fix zaproxy/zaproxy#1955 - Allow to AJAX spider a context
Fix zaproxy/zaproxy#1956 - Allow to AJAX spider as a user
Fix zaproxy/zaproxy#1957 - Allow URL to be pasted into AJAX Spider
dialogue
@thc202
Copy link
Member Author

thc202 commented Aug 26, 2016

Conflicts resolved.

@psiinon
Copy link
Member

psiinon commented Aug 30, 2016

Looks good

@kingthorin kingthorin merged commit 3845810 into zaproxy:master Aug 30, 2016
@thc202 thc202 deleted the spiderajax-context-user branch September 5, 2016 14:47
thc202 added a commit to thc202/crawljax that referenced this pull request Aug 1, 2017
Allow to control which of the URLs accessed can be crawled, for example,
to allow to crawl several domains or just some pages under a given
domain.

Related to zaproxy/zap-extensions#468 - spiderAjax: allow choose
context/user and set URL
thc202 added a commit to thc202/crawljax that referenced this pull request Aug 2, 2017
Allow to control which of the URLs accessed can be crawled, for example,
to allow to crawl several domains or just some pages under a given
domain.

Related to zaproxy/zap-extensions#468 - spiderAjax: allow choose
context/user and set URL
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants