mozilla :: #websectools

19 Apr 2017
08:10mgm_rain0rWould it be okay to add methods to
08:10mgm_rain0rI think I read somewhere code in org.parosproxy shouldn't be touched
08:12thc202it's ok to change paros code, you just need to add the ZAP comments
08:12thc202new classes should be in zap package though
08:16thc202what kind of changes do you want to do to that class? in theory you shouldn't need to call that class directly (better to work just with the Paros/ZAP code not the commons http client)
08:19mgm_rain0rI am looking for a way to group / manage the HTTP methods in a nice way ( )
08:19mgm_rain0rand I saw that there are already some HTTP methods
08:19mgm_rain0rI'm thinking about adding some more methods
08:19mgm_rain0rand then access them via the extension
08:23mgm_rain0rbut I could also create a new class with all the methods in the zap package
08:25thc202how would that be used? (compared to what we support now, just set the method to the HttpRequestHeader)
08:25mgm_rain0rfor the insecure http methods extension
08:26mgm_rain0rsee the comment of king thorin in the PR
08:27mgm_rain0rnevertheless, the class HttpMethodHelper is a mess and should be cleaned
08:30thc202ah, I got what you mean now, HttpRequestHeader is a better place to add the methods (which already has some of them)
08:31thc202(I guess what kingthorin's comment implies is to use constants instead of the hardcoded names)
08:34mgm_rain0rcan I add the webdav methods there?
08:35mgm_rain0rHmm I don't think they belong there :>
08:36thc202if those methods are not used in other places you can just define the constants in the scanner
08:37thc202but it would be fine to add them to that class (we already add a lot of non standard headers to HttpHeader)
08:38mgm_rain0rand .. how about creating a ticket to clean up this class:
08:39thc202there's already one to "clean up" the "network" classes ;)
08:40mgm_rain0rAt least removing all the commented code
08:41thc202fine by me (better a PR than an issue though ;)
11:25mgm_rain0rHm I wanted to create an extra PR for this commit:
11:25mgm_rain0rbut it's already under my previous PR
11:33thc202create a new branch and add that commit to it, then you can open the PR with just that (to remove that commit from the current PR just reset the branch to previous commit)
11:37thc202(create a new branch from develop)
11:42cyyou can also create a new branch from master and cherry-pick that commit to that branch
11:43thc202yes, but not master, develop ;)
11:43thc202mgm_rain0r, PRs are based on branches, that's why you need to create a new branch for the new PR
11:44thc202(and should be develop from "upstream" (or whatever the zaproxy remote is called), not your local develop branch)
11:58mgm_rain0rHi there kingthorin
11:58mgm_rain0rthc202, I'm actually cleaning up my branch mess
11:58mgm_rain0rat the moment
11:59kingthorinso you've got the scoop on feature branches and separate PRs, etc?
12:02mgm_rain0rnow, yes
12:03kingthorin@psiinon around?
12:03psiinonyep :)
12:03kingthorinshould I get Tanya to try out that mac test version?
12:04psiinonyeah - please
12:04psiinonwas going to ask the other reporter to try it too
12:04psiinonbut juggling too many things at once ;)
12:05kingthorinno worries, I just wanted to make sure that was good and not put my foot in anything :)
12:06psiinonI uploaded it for people to test :)
12:06psiinonjust need to tell a few people ;)
12:07kingthorinnoticed yesterday that when I opened an old session (maybe 2 wks or so) and deleted all the alerts they didn't actually delete. The tree was emptied in the UI but the API still reported them, until I used the deleteall API action then they were more gone. The site tree never seems to drop the flags though, even if I "refresh" from the right-click menu
12:08kingthorinwill work on re-creating the behavior and open a ticket for it
12:09mgm_rain0rI created now a "correct" PR (i.e. with an own branch) for the HTTP methods
12:10mgm_rain0rthis PR depends on it
12:13kingthorinit looks like you created a new branch based on an old branch instead of a clean (develop) the new PR still has the "Enable HTTP State" stuff
12:18kingthorinalso with the current setup zap-extensions has a bundled core zap.jar which is only (historically) upgraded at release time, so I'd suggest adding a TODO item to your extension PR stating that its awaiting a core update......vs leaving the extension PR open until after the next release
12:19kingthorinI gotta go, will try to check in later
12:19kingthorinhave a great day everyone
12:41thc202mgm_rain0r, you going to open a new PR or tweak 3426?
12:42mgm_rain0rI open a new one for the http methods in httprequestheader
12:44thc202kingthorin, (for the record) that's a known issue
12:46thc202mgm_rain0r, ok, may I suggest update 3426 instead?
12:58mgm_rain0r ok I try it
12:58mgm_rain0rhave to sort out my branch mess here
12:58mgm_rain0rI screwed up
13:00thc202ok, let me know if I can help (note you can always restore to a previous state, git reflog helps with that)
13:11mgm_rain0rthe problem was that I didnt create a new branch for the "move http session to options" task
13:11mgm_rain0rso I could not create a "clean" branch
13:11mgm_rain0rNow I got a branch for everything :)
13:13thc202you could have used the remote develop branch for that (http methods branch)
13:13thc202ok :)
13:15thc202you should be able to restore the PRs (unless you don't want to use develop for http session option, but for the other one it should be fine)
13:19mgm_rain0rHm I must have deleted the PRs
13:21thc202they are still there, if you push the branch httpmethods you should be able to reopen 3426
13:22mgm_rain0r"Everything up-to-date
13:22mgm_rain0r" :-/
13:22thc202to what command?
13:23thc202ah, the branch was already pushed, you should be able to open the PR now?
13:23mgm_rain0rI should
13:24mgm_rain0rI cant click on "Reopen"
13:27thc202ok, makes sense, the branch was completely changed, a new PR then
13:36mgm_rain0rI'm done
13:48mgm_rain0rnow meeeerge ;-)
13:50thc202note that, as kingthorin said, the zap-extensions PR should not use the new methods, those changes will have to wait for the core changes to be included in a main release
13:50thc202you can only include those changes if the add-on was targeting a weekly/dev release
16:32stephend|mtgpsiinon: forgot to specify a room; I'm in mine :-)
16:33psiinonah, just tried that, was obviously too early ;)
16:33stephend|mtgyeah, sorry, just got in
20 Apr 2017
No messages
Last message: 181 days ago