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

custompayloads: SAST Fixes (SonarLint), Options panel help, & drop ID #5853

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions addOns/custompayloads/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
### Changed
- Update minimum ZAP version to 2.15.0.
- Maintenance changes.
- Add help button to Options panel and add further detailed Help content.
- The superfluous ID element has been removed from the GUI and config.

## [0.13.0] - 2023-11-10
### Changed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@

import java.awt.Dimension;
import java.awt.Window;
import java.util.ArrayList;
import java.util.List;
import org.zaproxy.zap.utils.DisplayUtils;
import org.zaproxy.zap.view.StandardFieldsDialog;
Expand Down Expand Up @@ -95,7 +94,7 @@ private void createStringTextFieldForColumn(Column<T> column) {
private void createStringComboFieldForColumn(Column<T> column) {
EditableSelectColumn<T> selectColumn = (EditableSelectColumn<T>) column;
String value = column.getTypedValue(model);
ArrayList<String> selectableValues = selectColumn.getTypedSelectableValues(model);
List<String> selectableValues = selectColumn.getTypedSelectableValues(model);
this.addComboField(column.getNameKey(), selectableValues, value);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@
*/
package org.zaproxy.zap.extension.custompayloads;

public abstract class Column<T> {
abstract class Column<T> {
Class<?> columnClass;
String nameKey;

public Column(Class<?> columnClass, String nameKey) {
Column(Class<?> columnClass, String nameKey) {
this.columnClass = columnClass;
this.nameKey = nameKey;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@

public class CustomPayload implements EnableableInterface {

private int id;
private boolean enabled;
private String category;
private String payload;
Expand All @@ -33,20 +32,11 @@ public CustomPayload(String category, String payload) {
}

public CustomPayload(boolean enabled, String category, String payload) {
this(-1, enabled, category, payload);
}

public CustomPayload(int id, boolean enabled, String category, String payload) {
this.id = id;
this.enabled = enabled;
this.category = category;
this.payload = payload;
}

public void setId(int id) {
this.id = id;
}

public void setCategory(String category) {
this.category = category;
}
Expand All @@ -73,11 +63,7 @@ public String getPayload() {
return payload;
}

public int getId() {
return id;
}

public CustomPayload copy() {
return new CustomPayload(id, enabled, category, payload);
return new CustomPayload(enabled, category, payload);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public ArrayList<Object> getSelectableValues(CustomPayload payload) {
return categoryObjects;
}

private ExtensionCustomPayloads getExtension() {
private static ExtensionCustomPayloads getExtension() {
return Control.getSingleton()
.getExtensionLoader()
.getExtension(ExtensionCustomPayloads.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,13 @@

public final class CustomPayloadColumns {

private CustomPayloadColumns() {
// Nothing to do
}

public static List<Column<CustomPayload>> createColumns() {
ArrayList<Column<CustomPayload>> columns = new ArrayList<>();
columns.add(createEnabledColumn());
columns.add(createIdColumn());
columns.add(createCategoryColumn());
columns.add(createPayloadColumn());
return columns;
Expand All @@ -37,9 +40,8 @@ public static List<Column<CustomPayload>> createColumns() {
public static List<Column<CustomPayload>> createColumnsForOptionsTable() {
ArrayList<Column<CustomPayload>> columns = new ArrayList<>();
columns.add(createEnabledColumn());
columns.add(createIdColumn());
columns.add(createCategoryColumn().AsReadonly());
columns.add(createPayloadColumn().AsReadonly());
columns.add(createCategoryColumn().asReadonly());
columns.add(createPayloadColumn().asReadonly());
return columns;
}

Expand All @@ -62,16 +64,6 @@ public Object getValue(CustomPayload payload) {
};
}

private static Column<CustomPayload> createIdColumn() {
return new Column<CustomPayload>(Integer.class, "custompayloads.options.dialog.id") {

@Override
public Object getValue(CustomPayload payload) {
return payload.getId();
}
};
}

private static EditableColumn<CustomPayload> createPayloadColumn() {
return new EditableColumn<CustomPayload>(
String.class, "custompayloads.options.dialog.payload") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
*/
package org.zaproxy.zap.extension.custompayloads;

import java.util.ArrayList;
import java.util.List;
import java.util.Set;

Expand All @@ -29,7 +28,6 @@ public class CustomPayloadMultipleOptionsTableModel

private static final long serialVersionUID = 1L;
private List<CustomPayload> defaultPayloads;
private int nextPayloadId;

public CustomPayloadMultipleOptionsTableModel() {
super(CustomPayloadColumns.createColumnsForOptionsTable());
Expand All @@ -39,36 +37,16 @@ public void setDefaultPayloads(List<CustomPayload> defaultPayloads) {
this.defaultPayloads = defaultPayloads;
}

public void setNextPayloadId(int nextPayloadId) {
this.nextPayloadId = nextPayloadId;
}

public int getNextPayloadId() {
return nextPayloadId;
}

public void resetPayloadIds() {
nextPayloadId = 1;
for (CustomPayload payload : getElements()) {
setNextIdToPayload(payload);
}
if (this.getRowCount() > 0) {
fireTableRowsUpdated(0, getElements().size() - 1);
}
}

public void resetToDefaults() {
clear();
for (CustomPayload defaultPayload : defaultPayloads) {
CustomPayload newPayload = defaultPayload.copy();
setNextIdToPayload(newPayload);
addModel(newPayload);
}
}

public void addToTable(ArrayList<CustomPayload> payloads) {
public void addToTable(List<CustomPayload> payloads) {
for (CustomPayload payload : payloads) {
payload.setId(nextPayloadId++);
addModel(payload);
}
}
Expand All @@ -81,10 +59,6 @@ public void getPayloadsOfACategory(Set<String> payloads, String category) {
}
}

public void setNextIdToPayload(CustomPayload payload) {
payload.setId(nextPayloadId++);
}

public void addMissingDefaultPayloads() {
for (CustomPayload defaultPayload : defaultPayloads) {
boolean alreadyExisting = false;
Expand All @@ -98,7 +72,6 @@ public void addMissingDefaultPayloads() {

if (!alreadyExisting) {
CustomPayload newPayload = defaultPayload.copy();
setNextIdToPayload(newPayload);
addModel(newPayload);
}
}
Expand Down
kingthorin marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@ public class CustomPayloadsMultipleOptionsTablePanel

private static final String RESET_BUTTON =
Constant.messages.getString("custompayloads.options.button.reset");
private static final String RESET_ID_BUTTON =
Constant.messages.getString("custompayloads.options.button.resetIds");
private static final String ADD_MISSING_DEFAULTS_BUTTON =
Constant.messages.getString("custompayloads.options.button.addMissingDefaults");

Expand All @@ -67,7 +65,6 @@ public class CustomPayloadsMultipleOptionsTablePanel
LogManager.getLogger(CustomPayloadsMultipleOptionsTablePanel.class);

private JButton resetButton;
private JButton resetButtonId;
private JButton addMissingDefaultsButton;
private JButton fileButton;
private CustomPayloadMultipleOptionsTableModel tableModel;
Expand All @@ -79,7 +76,6 @@ public CustomPayloadsMultipleOptionsTablePanel(
addButtonSpacer();
addMissingDefaultsButton();
addResetButton();
addResetIdButton();
addButtonSpacer();
addPayloadFileButton();
getTable().setHorizontalScrollEnabled(true);
Expand All @@ -91,12 +87,6 @@ private void addMissingDefaultsButton() {
addButton(addMissingDefaultsButton);
}

private void addResetIdButton() {
resetButtonId = new JButton(RESET_ID_BUTTON);
resetButtonId.addActionListener(e -> tableModel.resetPayloadIds());
addButton(resetButtonId);
}

private void addResetButton() {
resetButton = new JButton(RESET_BUTTON);
resetButton.addActionListener(e -> tableModel.resetToDefaults());
Expand All @@ -107,7 +97,7 @@ private void addPayloadFileButton() {
fileButton = new JButton(ADD_MULTIPLE_PAYLOADS_BUTTON);
fileButton.addActionListener(
e -> {
CustomPayload multiplePayloads = new CustomPayload(-1, true, "", "");
CustomPayload multiplePayloads = new CustomPayload(true, "", "");
CustomMultiplePayloadDialog dialog =
new CustomMultiplePayloadDialog(
View.getSingleton().getOptionsDialog(null), multiplePayloads);
Expand Down Expand Up @@ -161,15 +151,14 @@ private void addPayloadFileButton() {

@Override
public CustomPayload showAddDialogue() {
CustomPayload payload = new CustomPayload(-1, true, "", "");
CustomPayload payload = new CustomPayload(true, "", "");
if (showDialog(payload)) {
tableModel.setNextIdToPayload(payload);
return payload;
}
return null;
}

private boolean showDialog(CustomPayload payload) {
private static boolean showDialog(CustomPayload payload) {
CustomPayloadDialog dialog =
new CustomPayloadDialog(
View.getSingleton().getOptionsDialog(null),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ public void initParam(Object obj) {
tableModel.clear();
tableModel.addModels(param.getPayloads());
tableModel.setDefaultPayloads(param.getDefaultPayloads());
tableModel.setNextPayloadId(param.getNextPayloadId());
tablePanel.setRemoveWithoutConfirmation(param.isConfirmRemoveToken());
}

Expand All @@ -67,7 +66,11 @@ public void saveParam(Object obj) throws Exception {
OptionsParam optionsParam = (OptionsParam) obj;
CustomPayloadsParam param = optionsParam.getParamSet(CustomPayloadsParam.class);
param.setPayloads(tableModel.getElements());
param.setNextPayloadId(tableModel.getNextPayloadId());
param.setConfirmRemoveToken(tablePanel.isRemoveWithoutConfirmation());
}

@Override
public String getHelpIndex() {
return "custompayloads.options";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,14 @@ public class CustomPayloadsParam extends AbstractParam {
CUSTOM_PAYLOADS_BASE_KEY + ".categories.category";
private static final String CATEGORY_NAME_KEY = "[@name]";

private static final String PAYLOAD_ID_KEY = "id";
private static final String PAYLOAD_KEY = "payload";
private static final String PAYLOAD_ENABLED_KEY = "enabled";

private static final String CONFIRM_REMOVE_PAYLOAD_KEY =
CUSTOM_PAYLOADS_BASE_KEY + ".confirmRemoveToken";
private static final String NEXT_PAYLOAD_ID_KEY = CUSTOM_PAYLOADS_BASE_KEY + ".nextPayloadId";

private Map<String, PayloadCategory> payloadCategories;
private boolean confirmRemoveToken;
private int nextPayloadId = 1;

public CustomPayloadsParam() {
payloadCategories = new HashMap<>();
Expand All @@ -63,7 +60,6 @@ private void loadFromConfig() {
HierarchicalConfiguration rootConfig = (HierarchicalConfiguration) getConfig();
loadPayloadsFromConfig(rootConfig);
loadConfirmRemoveTokenFromConfig(rootConfig);
loadNextPayloadIdFromConfig(rootConfig);
initializeWithDefaultsIfPayloadsAreEmpty();
}

Expand All @@ -73,14 +69,12 @@ private void initializeWithDefaultsIfPayloadsAreEmpty() {
resetDefaults(category);
}
}
setNextPayloadId(nextPayloadId);
}

private void resetDefaults(PayloadCategory category) {
private static void resetDefaults(PayloadCategory category) {
List<CustomPayload> payloads = new ArrayList<>(category.getDefaultPayloads().size());
for (CustomPayload defaultPayload : category.getDefaultPayloads()) {
CustomPayload payload = defaultPayload.copy();
payload.setId(nextPayloadId++);
payloads.add(payload);
}
category.setPayloads(payloads);
Expand All @@ -95,10 +89,9 @@ private void loadPayloadsFromConfig(HierarchicalConfiguration rootConfig) {
String cat = category.getString(CATEGORY_NAME_KEY);
List<CustomPayload> payloads = new ArrayList<>();
for (HierarchicalConfiguration sub : fields) {
int id = sub.getInt(PAYLOAD_ID_KEY);
boolean isEnabled = sub.getBoolean(PAYLOAD_ENABLED_KEY);
String payload = sub.getString(PAYLOAD_KEY, "");
payloads.add(new CustomPayload(id, isEnabled, cat, payload));
payloads.add(new CustomPayload(isEnabled, cat, payload));
}
payloadCategories.put(cat, new PayloadCategory(cat, Collections.emptyList(), payloads));
}
Expand All @@ -108,39 +101,6 @@ private void loadConfirmRemoveTokenFromConfig(HierarchicalConfiguration rootConf
confirmRemoveToken = rootConfig.getBoolean(CONFIRM_REMOVE_PAYLOAD_KEY, true);
}

private void loadNextPayloadIdFromConfig(HierarchicalConfiguration rootConfig) {
int maxUsedPayloadId = getMaxUsedPayloadId();
nextPayloadId = rootConfig.getInteger(NEXT_PAYLOAD_ID_KEY, 1);
if (nextPayloadId <= maxUsedPayloadId) {
setNextPayloadId(maxUsedPayloadId + 1);
}
}

public int getNextPayloadId() {
return nextPayloadId;
}

public void setNextPayloadId(int id) {
nextPayloadId = id;
saveNextPayloadId();
}

private void saveNextPayloadId() {
getConfig().setProperty(NEXT_PAYLOAD_ID_KEY, Integer.valueOf(nextPayloadId));
}

private int getMaxUsedPayloadId() {
int maxUsedPayloadId = 0;
for (PayloadCategory category : payloadCategories.values()) {
for (CustomPayload payload : category.getPayloads()) {
if (maxUsedPayloadId < payload.getId()) {
maxUsedPayloadId = payload.getId();
}
}
}
return maxUsedPayloadId;
}

public List<CustomPayload> getPayloads() {
ArrayList<CustomPayload> clonedPayloads = new ArrayList<>();
for (PayloadCategory category : payloadCategories.values()) {
Expand Down Expand Up @@ -178,9 +138,6 @@ private void savePayloadsToConfig() {
String elementBaseKey =
catElementBaseKey + ".payloads." + PAYLOAD_KEY + "(" + i + ").";
CustomPayload payload = payloads.get(i);
getConfig()
.setProperty(
elementBaseKey + PAYLOAD_ID_KEY, Integer.valueOf(payload.getId()));
getConfig()
.setProperty(
elementBaseKey + PAYLOAD_ENABLED_KEY,
Expand Down
Loading