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

- AbstractColumnDialog > Use List interface vs ArrayList. - Column > Reduce visibility. - CustomPayload > Remove ID and related handling. - CustomPayloadColumns > Add private constructor to hide implicit one. Remove ID and related handling. - CustomPayloadMultipleOptionsTableModel > Use List interface vs ArrayList for params. Remove ID and related handling. - CustomPayloadsCategoryColumn > Make getExtension static. - CustomPayloadsMultipleOptionsTablePanel > Make showDialog static. Remove ID and related handling. - CustomPayloadsParam > Remove ID and related handling. - EditableColumn > Reduce visibility. - EditableSelectColumn > Reduce visibility, adjust method naming to proper Java camelCase. - CustomPayloadsApiUnitTest > Remove unused param API.RequestType, and pointless declared throw on shouldHavePrefix. - Messages.properties > Remove no longer needed ID key value pair. - CHANGELOG > Already has a maint note, added a note about the Options panel help button, and one about ID. - CustomPayloadsOptionsPanel > Added overridden getHelpIndex method. - Help files > Updated to include more detailed info about the functionality. Signed-off-by: kingthorin <[email protected]>
zaproxy · Nov 11, 2024 · 543e76b · 543e76b
1 parent e70e940
commit 543e76b
Show file tree

Hide file tree

Showing 18 changed files with 90 additions and 143 deletions.
diff --git a/addOns/custompayloads/CHANGELOG.md b/addOns/custompayloads/CHANGELOG.md
@@ -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

diff --git a/...payloads/src/main/java/org/zaproxy/zap/extension/custompayloads/AbstractColumnDialog.java b/...payloads/src/main/java/org/zaproxy/zap/extension/custompayloads/AbstractColumnDialog.java
@@ -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;
@@ -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);
     }
 

diff --git a/addOns/custompayloads/src/main/java/org/zaproxy/zap/extension/custompayloads/Column.java b/addOns/custompayloads/src/main/java/org/zaproxy/zap/extension/custompayloads/Column.java
@@ -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;
     }

diff --git a/.../custompayloads/src/main/java/org/zaproxy/zap/extension/custompayloads/CustomPayload.java b/.../custompayloads/src/main/java/org/zaproxy/zap/extension/custompayloads/CustomPayload.java
@@ -23,7 +23,6 @@
 
 public class CustomPayload implements EnableableInterface {
 
-    private int id;
     private boolean enabled;
     private String category;
     private String payload;
@@ -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;
     }
@@ -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);
     }
 }
diff --git a/...s/src/main/java/org/zaproxy/zap/extension/custompayloads/CustomPayloadCategoryColumn.java b/...s/src/main/java/org/zaproxy/zap/extension/custompayloads/CustomPayloadCategoryColumn.java
@@ -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);

diff --git a/...payloads/src/main/java/org/zaproxy/zap/extension/custompayloads/CustomPayloadColumns.java b/...payloads/src/main/java/org/zaproxy/zap/extension/custompayloads/CustomPayloadColumns.java
@@ -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;
@@ -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;
     }
 
@@ -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") {

diff --git a/...java/org/zaproxy/zap/extension/custompayloads/CustomPayloadMultipleOptionsTableModel.java b/...java/org/zaproxy/zap/extension/custompayloads/CustomPayloadMultipleOptionsTableModel.java
@@ -19,7 +19,6 @@
  */
 package org.zaproxy.zap.extension.custompayloads;
 
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Set;
 
@@ -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());
@@ -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);
         }
     }
@@ -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;
@@ -98,7 +72,6 @@ public void addMissingDefaultPayloads() {
 
             if (!alreadyExisting) {
                 CustomPayload newPayload = defaultPayload.copy();
-                setNextIdToPayload(newPayload);
                 addModel(newPayload);
             }
         }

diff --git a/...ava/org/zaproxy/zap/extension/custompayloads/CustomPayloadsMultipleOptionsTablePanel.java b/...ava/org/zaproxy/zap/extension/custompayloads/CustomPayloadsMultipleOptionsTablePanel.java
@@ -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");
 
@@ -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;
@@ -79,7 +76,6 @@ public CustomPayloadsMultipleOptionsTablePanel(
         addButtonSpacer();
         addMissingDefaultsButton();
         addResetButton();
-        addResetIdButton();
         addButtonSpacer();
         addPayloadFileButton();
         getTable().setHorizontalScrollEnabled(true);
@@ -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());
@@ -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);
@@ -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),

diff --git a/...ds/src/main/java/org/zaproxy/zap/extension/custompayloads/CustomPayloadsOptionsPanel.java b/...ds/src/main/java/org/zaproxy/zap/extension/custompayloads/CustomPayloadsOptionsPanel.java
@@ -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());
     }
 
@@ -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";
+    }
 }
diff --git a/...mpayloads/src/main/java/org/zaproxy/zap/extension/custompayloads/CustomPayloadsParam.java b/...mpayloads/src/main/java/org/zaproxy/zap/extension/custompayloads/CustomPayloadsParam.java
@@ -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<>();
@@ -63,7 +60,6 @@ private void loadFromConfig() {
         HierarchicalConfiguration rootConfig = (HierarchicalConfiguration) getConfig();
         loadPayloadsFromConfig(rootConfig);
         loadConfirmRemoveTokenFromConfig(rootConfig);
-        loadNextPayloadIdFromConfig(rootConfig);
         initializeWithDefaultsIfPayloadsAreEmpty();
     }
 
@@ -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);
@@ -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));
         }
@@ -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()) {
@@ -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,