[RFC][Icedtea-Web]: Ensure all ports entered are numeric values

[Omair Majid]

On 03/21/2011 01:51 PM, Andrew Su wrote:

> v1 does not alert the user (silently ignore input). v2 alerts the
> user via popup.
>

Only v2 will be applied, right?

> 20110321_number_only_v1.patch
>
>
> diff -r 466ad8570145 netx/net/sourceforge/jnlp/controlpanel/NetworkSettingsPanel.java
> --- a/netx/net/sourceforge/jnlp/controlpanel/NetworkSettingsPanel.java	Thu Mar 17 15:19:39 2011 -0400
> +++ b/netx/net/sourceforge/jnlp/controlpanel/NetworkSettingsPanel.java	Mon Mar 21 13:39:02 2011 -0400
> @@ -21,6 +21,7 @@
>   import java.awt.BorderLayout;
>   import java.awt.CardLayout;
>   import java.awt.Component;
> +import java.awt.Dialog;
>   import java.awt.Dimension;
>   import java.awt.FlowLayout;
>   import java.awt.GridBagConstraints;
> @@ -35,10 +36,15 @@
>   import javax.swing.ButtonGroup;
>   import javax.swing.JButton;
>   import javax.swing.JCheckBox;
> +import javax.swing.JDialog;
>   import javax.swing.JLabel;
> +import javax.swing.JOptionPane;
>   import javax.swing.JPanel;
>   import javax.swing.JRadioButton;
>   import javax.swing.JTextField;

Dont know if you are going to apply this patch or not, but some of the 
imports are unused (including Dialog, JDialog, and JOptionPane). Please 
remove them.

> +import javax.swing.text.AttributeSet;
> +import javax.swing.text.BadLocationException;
> +import javax.swing.text.PlainDocument;
>
>   import net.sourceforge.jnlp.config.DeploymentConfiguration;
>   import net.sourceforge.jnlp.runtime.Translator;
> @@ -113,8 +119,10 @@
>           final JTextField addressField = new JTextField(config.getProperty(properties[1]), 10);
>           addressField.getDocument().addDocumentListener(new DocumentAdapter(config, properties[1]));
>
> -        final JTextField portField = new JTextField(config.getProperty(properties[2]), 3);
> +        final JTextField portField = new JTextField(5);
> +        portField.setDocument(NetworkSettingsPanel.getNumberDocument(this));
>           portField.getDocument().addDocumentListener(new DocumentAdapter(config, properties[2]));
> +        portField.setText(config.getProperty(properties[2]));
>
>           // Create the button which allows setting of other types of proxy.
>           JButton advancedProxyButton = new JButton(Translator.R("NSAdvanced") + "...");
> @@ -254,4 +262,28 @@
>                   enablePanel(panel, false);
>           }
>       }
> +
> +    /**
> +     * Creates a PlainDocument that only take numbers if it will create a valid port number.
> +     * @return PlainDocument which will ensure numeric values only and is a valid port number.
> +     */
> +    public static PlainDocument getNumberDocument(final Component c){
> +        return new PlainDocument(){
> +            public void insertString(int offs, String str, AttributeSet a) throws BadLocationException {
> +                if (str != null) {
> +                    try {
> +                        Integer.valueOf(str);
> +                        int val = Integer.valueOf(this.getText(0, this.getLength()) + str);
> +                        if (val<  1 || val>  65535) { // Invalid port number if true
> +                            throw new NumberFormatException("Invalid port number");
> +                        }
> +                        super.insertString(offs, str, a);
> +                    } catch (Exception e) {
> +                        // Just don't insert.
> +                    }
> +                }
> +                return;
> +            }
> +        };
> +    }

What about remove()? How is deleting port numbers handled?

>   }
> diff -r 466ad8570145 netx/net/sourceforge/jnlp/controlpanel/TemporaryInternetFilesPanel.java
> --- a/netx/net/sourceforge/jnlp/controlpanel/TemporaryInternetFilesPanel.java	Thu Mar 17 15:19:39 2011 -0400
> +++ b/netx/net/sourceforge/jnlp/controlpanel/TemporaryInternetFilesPanel.java	Mon Mar 21 13:39:02 2011 -0400
> @@ -19,6 +19,7 @@
>   package net.sourceforge.jnlp.controlpanel;
>
>   import java.awt.BorderLayout;
> +import java.awt.Component;
>   import java.awt.FlowLayout;
>   import java.awt.GridBagConstraints;
>   import java.awt.GridBagLayout;
> @@ -33,6 +34,7 @@
>   import javax.swing.JComponent;
>   import javax.swing.JFileChooser;
>   import javax.swing.JLabel;
> +import javax.swing.JOptionPane;
>   import javax.swing.JPanel;
>   import javax.swing.JSlider;
>   import javax.swing.JSpinner;
> @@ -40,6 +42,9 @@
>   import javax.swing.SpinnerNumberModel;
>   import javax.swing.event.ChangeEvent;
>   import javax.swing.event.ChangeListener;
> +import javax.swing.text.AttributeSet;
> +import javax.swing.text.BadLocationException;
> +import javax.swing.text.PlainDocument;
>
>   import net.sourceforge.jnlp.config.DeploymentConfiguration;
>   import net.sourceforge.jnlp.runtime.Translator;
>

The only changes to this file are in imports? Please revert this file.

>
> 20110321_number_only_v2.patch
>
> diff -r 466ad8570145 netx/net/sourceforge/jnlp/controlpanel/NetworkSettingsPanel.java
> --- a/netx/net/sourceforge/jnlp/controlpanel/NetworkSettingsPanel.java	Thu Mar 17 15:19:39 2011 -0400
> +++ b/netx/net/sourceforge/jnlp/controlpanel/NetworkSettingsPanel.java	Mon Mar 21 13:38:47 2011 -0400
> @@ -254,4 +262,32 @@
>                   enablePanel(panel, false);
>           }
>       }
> +
> +    /**
> +     * Creates a PlainDocument that only take numbers if it will create a valid port number.
> +     * @return PlainDocument which will ensure numeric values only and is a valid port number.
> +     */
> +    public static PlainDocument getNumberDocument(final Component c){
> +        return new PlainDocument(){
> +            public void insertString(int offs, String str, AttributeSet a) throws BadLocationException {
> +                if (str != null) {
> +                    try {
> +                        Integer.valueOf(str);
> +                        int val = Integer.valueOf(this.getText(0, this.getLength()) + str);
> +                        if (val<  1 || val>  65535) { // Invalid port number if true
> +                            throw new NumberFormatException("Invalid port number");
> +                        }
> +                        super.insertString(offs, str, a);
> +                    } catch (Exception e) {
> +                        JOptionPane jop = new JOptionPane("Invalid port number given.\n[Valid port numbers are 1-65536]", JOptionPane.WARNING_MESSAGE);

Please add this string to Messages.properties

> +                        JDialog dialog = jop.createDialog(null, "Error on input");
> +                        dialog.setModalityType(Dialog.ModalityType.APPLICATION_MODAL);
> +                        dialog.setVisible(true);
> +                        dialog.dispose();

Any reason not to use JOptionPane.showMessageDialog?

> +                    }
> +                }
> +                return;
> +            }
> +        };
> +    }
>   }
> diff -r 466ad8570145 netx/net/sourceforge/jnlp/controlpanel/TemporaryInternetFilesPanel.java
> --- a/netx/net/sourceforge/jnlp/controlpanel/TemporaryInternetFilesPanel.java	Thu Mar 17 15:19:39 2011 -0400
> +++ b/netx/net/sourceforge/jnlp/controlpanel/TemporaryInternetFilesPanel.java	Mon Mar 21 13:38:47 2011 -0400
> @@ -19,6 +19,7 @@
>   package net.sourceforge.jnlp.controlpanel;
>
>   import java.awt.BorderLayout;
> +import java.awt.Component;
>   import java.awt.FlowLayout;
>   import java.awt.GridBagConstraints;
>   import java.awt.GridBagLayout;
> @@ -33,6 +34,7 @@
>   import javax.swing.JComponent;
>   import javax.swing.JFileChooser;
>   import javax.swing.JLabel;
> +import javax.swing.JOptionPane;
>   import javax.swing.JPanel;
>   import javax.swing.JSlider;
>   import javax.swing.JSpinner;
> @@ -40,6 +42,9 @@
>   import javax.swing.SpinnerNumberModel;
>   import javax.swing.event.ChangeEvent;
>   import javax.swing.event.ChangeListener;
> +import javax.swing.text.AttributeSet;
> +import javax.swing.text.BadLocationException;
> +import javax.swing.text.PlainDocument;
>

Only changes are imports; please revert.

Cheers,
Omair