brk: migrate from core

[aryangupta701]

Overview

Part of zaproxy/zaproxy#8153

• migrate brk form core to addon
• update plugnhack to use brk addon
• update websocket to use brk addon
• update brk addon to use network addon

Related Issues

Specify any related issues or pull requests by linking to them.

Checklist

• Update help
• Update changelog
• Run ./gradlew spotlessApply for code formatting
• Write tests
• Check code coverage
• Sign-off commits
• Squash commits
• Use a descriptive title

For more details, please refer to the developer rules and guidelines.

I have read the CLA Document and I hereby sign the CLA

[thc202]

You need to add the new add-on/project to the settings.gradle.kts file:

zap-extensions/settings.gradle.kts

Line 27 in ac4cae7

var addOns = listOf(

Note that network should not depend on the new add-on.

[aryangupta701]

why should the network not depend on the break addon? I mean it will be using it right?

[aryangupta701]

also how should I use this class? WithConfigsTest I am not able to use it directly I think I will have to create a clone for it in zap-extensions also?

[thc202]

#5213 (comment)

The network add-on does not need the break functionality to work, it could be an optional dependency but even in that case there's no networking functionality that would depend on it. What should be done is change the network add-on to allow to set the serializer and the break add-on would set it.

#5213 (comment)

You need to adapt the tests to use TestUtils from the testutils project.

[aryangupta701]

can plugnhack and websocket depend on break addon? @thc202

[thc202]

Yes, although it could be an optional dependency.

[aryangupta701]

can you share an example of how to add an optional dependency?

[thc202]

zap-extensions/addOns/quickstart/quickstart.gradle.kts

Lines 53 to 64 in ac4cae7

	register("org.zaproxy.zap.extension.quickstart.spider.ExtensionQuickStartSpider") {
	classnames {
	allowed.set(listOf("org.zaproxy.zap.extension.quickstart.spider"))
	}
	dependencies {
	addOns {
	register("spider") {
	version.set(">=0.1.0")
	}
	}
	}
	}

[aryangupta701]

are there any any existing help pages for brk extension? @thc202

[thc202]

#5213 (comment)

You need to create a new extension in the corresponding add-ons (in the previous example, ExtensionQuickStartSpider which accesses the spider add-on). The classnames should allow classes (though simpler with a package) of the add-on itself not of the dependency.

#5213 (comment)

Yes, they are all currently in the zap-core-help repo (e.g. https://github.com/zaproxy/zap-core-help/blob/73a4fd1d5d2ae72fef6607ff35e3b22e49ea667a/addOns/help/src/main/javahelp/contents/ui/dialogs/addbreak.html).

[aryangupta701]

I am planning to do this in 4 different PRs as updated in this PR's description. Will it be fine?

[psiinon]

Thats fine as long as the changes are consistant and dont break things inbetween the PRs.

[thc202]

Yes, similar to what was done for the spider zaproxy/zaproxy#7335.

[aryangupta701]

I am not sure why CLA is not working

[psiinon]

I am not sure why CLA is not working

Me neither :/
Your details are not being added to https://github.com/zaproxy/cla/blob/main/signatures/version1/cla.json

[aryangupta701]

I have read the CLA Document and I hereby sign the CLA

[thc202]

Add just the agreeing comment and no other comments after (we will have to run the check manually).

[aryangupta701]

I have read the CLA Document and I hereby sign the CLA

[thc202]

CLA is now passing, it was hitting the default comments limit. Also removed the dummy test commit.

[aryangupta701]

I think now is the good time for a quick review

[thc202]

Did you mean to push more changes? Previous comments are still pending (e.g. #5213 (comment)).

[aryangupta701]

oops forgot that. Thanks

[aryangupta701]

shall I now move on to other addons to migrate to brk addon?

[thc202]

IMO better finish the changes in the brk add-on before that.

[aryangupta701]

what changes are left for break add-on?

[thc202]

At least the following:

• Second part of brk: migrate from core #5213 (comment)
• Finish help migration (add a main page and move all core help content)
• Use other order and name for the extension (currently failing with class cast exceptions as existing add-ons try to use it)
• Don't throw RuntimeException if not able to find core interface (although that will be guarded by the deprecation check, it should still handle that case gracefully)
• The extension should be unloadable (also worth double check that all is)
• BreakpointsParam should be changed to extend VersionedAbstractParam
• Do not rely on core resource messages for the API's descriptions
• Remove since JavaDoc tags, no longer apply since this is new code
• Update help keys to use the ones of the add-on (e.g. calls to ExtensionHelp.enableHelpKey(...) and overrides of getHelpIndex())

[kingthorin]

@aryangupta701 are you able to keep going with this?