brk: migrate from core

[aryangupta701]

Overview

Part of zaproxy/zaproxy#8153

• [x] 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: https://github.com/zaproxy/zap-extensions/blob/ac4cae7b7c237afab4e72f824e0210018dbc6d7f/settings.gradle.kts#L27

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]

https://github.com/zaproxy/zap-extensions/pull/5213#issuecomment-1890910636

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.

https://github.com/zaproxy/zap-extensions/pull/5213#issuecomment-1890912818

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]

https://github.com/zaproxy/zap-extensions/blob/ac4cae7b7c237afab4e72f824e0210018dbc6d7f/addOns/quickstart/quickstart.gradle.kts#L53-L64

[aryangupta701]

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

[thc202]

https://github.com/zaproxy/zap-extensions/pull/5213#discussion_r1452103377

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.

https://github.com/zaproxy/zap-extensions/pull/5213#issuecomment-1891721849

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.

[aryangupta701]

Sure Simon.

I wanted to know how do we generate the help files for all the languages. What is the command for that?

[thc202]

They are added automatically when syncing from Crowdin.

[thc202]

https://github.com/zaproxy/zap-extensions/pull/5213#issuecomment-1891812419

It might just be a matter of updating the task text, but the network add-on should not use the brk add-on, it should be the other way around.

[aryangupta701]

I think it will be worth getting a review for the first task now i.e. migrate brk form core to addon. So that I can continue with the updation of other addons as well.

[thc202]

Quick comments:

• The functionality provided by the add-on should be added conditionally based on core's state, installing the add-on in dev/weekly should not cause any warns/errors (before and after the core changes). Same for main release but for that we can just require minimum of 2.15.
• The help is incomplete (e.g. helpset, index, toc) and it's missing core help pages.
• The extension should be unloadable (also worth double check that all is).

[aryangupta701]

I am not able to understand how the help pages should be imported. Since the break page on the core help page links to multiple pages and those pages then further link to many more so this would not be trivial to add

[aryangupta701]

re : The functionality provided by the add-on should be added conditionally based on core's state,

Where should this condition be present? Like in which file?

[thc202]

https://github.com/zaproxy/zap-extensions/pull/5213#issuecomment-1925477396

See e.g. zaproxy/zap-core-help#450 and #3848.

https://github.com/zaproxy/zap-extensions/pull/5213#issuecomment-1925478921

See e.g. zaproxy/zaproxy#7335 and https://github.com/zaproxy/zap-extensions/pull/3848/files#diff-1bce03d563372eb477beadc27d3a09f7581995a416dd3a09f60d47b920fc4671R76

[aryangupta701]

thank you. Sorry for not getting much time but will complete this very soon

[github-actions[bot]]

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

[kingthorin]

Please remove the merge and do a rebase.

[aryangupta701]

is there any automated way to generate these three files in java help?

1. index.xml
2. map.jhm
3. loc.xml

[kingthorin]

Nope.

[aryangupta701]

I don't know much about these files and how they work. Could you provide a little guidance for it please

[kingthorin]

Just model it based on another add-on, it should only require updating the add-on name (vs others).

[aryangupta701]

okay thank you

[kingthorin]

Do what it says: https://github.com/zaproxy/zap-extensions/pull/5213#issuecomment-1944776621

[aryangupta701]

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