Malformed PortRange or ThirdPartyAuthConfig trigger OnPolicyError.

remoting/base/port_range.h

Index: remoting/base/port_range.h
diff --git a/remoting/base/port_range.h b/remoting/base/port_range.h
new file mode 100644
index 0000000000000000000000000000000000000000..98e45c9c284cf721268fe37d64e903e2a81827e3
--- /dev/null
+++ b/remoting/base/port_range.h
@@ -0,0 +1,35 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef REMOTING_BASE_PORT_RANGE_H_

[Sergey Ulanov, 2015/02/27 03:05:19]

does this need to be in /base instead of /protocol?

[Łukasz Anforowicz, 2015/02/27 18:36:12]

I can move to "protocol".

+#define REMOTING_BASE_PORT_RANGE_H_
+
+#include <ostream>
+#include <string>
+
+#include "base/basictypes.h"
+
+namespace remoting {
+
+// Wrapper for an already parsed value of UdpPortRange policy.
+struct PortRange {

[Sergey Ulanov, 2015/02/27 03:05:19]

Please add a constructor to initialize to (0, 0)

[Łukasz Anforowicz, 2015/02/27 18:36:12]

Hmmm.  Initially I wanted to respond by saying that this is more complicated -
that there are 3 cases:
- Uninitialized.  Currently no representation.
- Initialized from the policy.  Currently represented by min_port==max_port==0.
- Initialized from the policy.  Represented by whatever was read from the
policy.

And I wanted to keep "uninitialized" separate (to allow DCHECK-ing against it). 
But then I realized that the default constructor of std::string initializes it
to an empty string, not to a special "you forgot to initialize this string"
value.

So yes - I should just initialize to 0, 0.

+  // Both |min_port| and |max_port| are inclusive.
+  uint16 min_port;
+  uint16 max_port;
+
+  inline bool is_empty() const { return (min_port == 0) && (max_port == 0); }

[Sergey Ulanov, 2015/02/27 03:05:19]

Maybe call this is_null(), or is_open()?
"empty range" means "a range that doesn't include any values".

[Łukasz Anforowicz, 2015/02/27 18:36:12]

Good point.  Thanks.  I'll also rename ThirdPartyAuthConfig::is_empty.

+
+  // Parse string in the form "<min_port>-<max_port>". E.g. "12400-12409".
+  // Returns true if string was parsed successfuly.
+  //
+  // Returns false and doesn't modify |out| if parsing fails (i.e. when
+  // |port_range| doesn't represent a valid port range).
+  static bool Parse(const std::string& port_range, PortRange* out);
+};
+
+std::ostream& operator<<(std::ostream& os, const PortRange& port_range);
+
+}  // namespace remoting
+
+#endif  // REMOTING_BASE_PORT_RANGE_H_