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

[Omair Majid]

On 04/05/2011 04:27 PM, Andrew Su wrote:
> ping.
>
> I've updated file to apply cleanly to head.
>
>
> 20110405_number_only_v4.patch
>
>
> diff -r eea730466b87 netx/net/sourceforge/jnlp/controlpanel/AdvancedProxySettingsPane.java
> --- a/netx/net/sourceforge/jnlp/controlpanel/AdvancedProxySettingsPane.java	Tue Apr 05 14:38:22 2011 -0400
> +++ b/netx/net/sourceforge/jnlp/controlpanel/AdvancedProxySettingsPane.java	Tue Apr 05 16:26:59 2011 -0400
> @@ -114,30 +114,38 @@
>           // This addresses the HTTP proxy settings.
>           JLabel http = new JLabel(Translator.R("APSLabelHTTP") + ":");
>           final JTextField httpAddressField = new JTextField(fields[0]);
> -        final JTextField httpPortField = new JTextField(fields[1]);
> +        final JTextField httpPortField = new JTextField();
> +        httpPortField.setDocument(NetworkSettingsPanel.getNumberDocument(this));
>           httpAddressField.getDocument().addDocumentListener(new DocumentAdapter(fields, 0));
>           httpPortField.getDocument().addDocumentListener(new DocumentAdapter(fields, 1));
> +        httpPortField.setText(fields[1]);
>
>           // This addresses the HTTPS proxy settings.
>           JLabel secure = new JLabel(Translator.R("APSLabelSecure") + ":");
>           final JTextField secureAddressField = new JTextField(fields[2]);
> -        final JTextField securePortField = new JTextField(fields[3]);
> +        final JTextField securePortField = new JTextField();
> +        securePortField.setDocument(NetworkSettingsPanel.getNumberDocument(this));
>           secureAddressField.getDocument().addDocumentListener(new DocumentAdapter(fields, 2));
>           securePortField.getDocument().addDocumentListener(new DocumentAdapter(fields, 3));
> +        securePortField.setText(fields[3]);
>
>           // This addresses the FTP proxy settings.
>           JLabel ftp = new JLabel(Translator.R("APSLabelFTP") + ":");
>           final JTextField ftpAddressField = new JTextField(fields[4]);
> -        final JTextField ftpPortField = new JTextField(fields[5]);
> +        final JTextField ftpPortField = new JTextField();
> +        ftpPortField.setDocument(NetworkSettingsPanel.getNumberDocument(this));
>           ftpAddressField.getDocument().addDocumentListener(new DocumentAdapter(fields, 4));
>           ftpPortField.getDocument().addDocumentListener(new DocumentAdapter(fields, 5));
> +        ftpPortField.setText(fields[5]);
>
>           // This addresses the Socks proxy settings.
>           JLabel socks = new JLabel(Translator.R("APSLabelSocks") + ":");
>           final JTextField socksAddressField = new JTextField(fields[6]);
> -        final JTextField socksPortField = new JTextField(fields[7]);
> +        final JTextField socksPortField = new JTextField();
> +        socksPortField.setDocument(NetworkSettingsPanel.getNumberDocument(this));
>           socksAddressField.getDocument().addDocumentListener(new DocumentAdapter(fields, 6));
>           socksPortField.getDocument().addDocumentListener(new DocumentAdapter(fields, 7));
> +        socksPortField.setText(fields[7]);
>
>           JCheckBox sameProxyForAll = new JCheckBox(Translator.R("APSSameProxyForAllProtocols"), Boolean.parseBoolean(fields[8]));
>           sameProxyForAll.addItemListener(new ItemListener() {
> diff -r eea730466b87 netx/net/sourceforge/jnlp/controlpanel/NetworkSettingsPanel.java
> --- a/netx/net/sourceforge/jnlp/controlpanel/NetworkSettingsPanel.java	Tue Apr 05 14:38:22 2011 -0400
> +++ b/netx/net/sourceforge/jnlp/controlpanel/NetworkSettingsPanel.java	Tue Apr 05 16:26:59 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;
> +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,29 @@
>                   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){

Hm... what is c? Is it used in this method? If it's something left over 
from a previous patch, it might be best just to delete it. I would also 
prefer it if the method name indicated that it's meant for port numbers, 
not general numbers.

> +        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.showMessageDialog(null, Translator.R("CPInvalidPort"), Translator.R("CPInvalidPortTitle")
> +                                , JOptionPane.WARNING_MESSAGE);
> +                    }
> +                }
> +                return;
> +            }
> +        };
> +    }
>   }
> diff -r eea730466b87 netx/net/sourceforge/jnlp/resources/Messages.properties
> --- a/netx/net/sourceforge/jnlp/resources/Messages.properties	Tue Apr 05 14:38:22 2011 -0400
> +++ b/netx/net/sourceforge/jnlp/resources/Messages.properties	Tue Apr 05 16:26:59 2011 -0400
> @@ -407,6 +407,8 @@
>
>   # Control Panel - Misc.
>   CPJRESupport=IcedTea-Web currently does not support the use of multiple JREs.
> +CPInvalidPort=Invalid port number given.\n[Valid port numbers are 1-65535]
> +CPInvalidPortTitle=Error on input.
>
>   # command line control panel
>   CLNoInfo=No information avaiable (is this a valid option?).

Everything else looks fine.

Cheers,
Omair