Use correct cookie date parsing for cookie 'expires' field.

Use correct cookie date parsing for cookie 'expires' field.


BUG=

Committed: https://code.google.com/p/dart/source/detail?r=11783

[Søren Gjesse, 2012-09-04 06:51:57]

lgtm

https://chromiumcodereview.appspot.com/10909057/diff/1/runtime/bin/http_utils...
File runtime/bin/http_utils.dart (right):

https://chromiumcodereview.appspot.com/10909057/diff/1/runtime/bin/http_utils...
runtime/bin/http_utils.dart:253: throw new HttpException("Invalid HTTP date
$date");
HTTP -> cookie

https://chromiumcodereview.appspot.com/10909057/diff/1/runtime/bin/http_utils...
runtime/bin/http_utils.dart:257: return position >= date.length;
Shouldn't this be just ==?

https://chromiumcodereview.appspot.com/10909057/diff/1/runtime/bin/http_utils...
runtime/bin/http_utils.dart:260: bool isDelimiter(String s) {
How about passing the index here and use s.charCodeAt(index) instead of creating
single character substrings.

https://chromiumcodereview.appspot.com/10909057/diff/1/runtime/bin/http_utils...
runtime/bin/http_utils.dart:304: while (!isEnd() &&
isNonDelimiter(date[position])) {
How about saving position before the loop, just incrementing position in the
loop and making a substring after the loop?

https://chromiumcodereview.appspot.com/10909057/diff/1/runtime/bin/http_utils...
runtime/bin/http_utils.dart:317: if (token.length < 1) return null;
Shouldn't this be a continue instead of return null?

https://chromiumcodereview.appspot.com/10909057/diff/1/tests/standalone/io/ht...
File tests/standalone/io/http_date_test.dart (right):

https://chromiumcodereview.appspot.com/10909057/diff/1/tests/standalone/io/ht...
tests/standalone/io/http_date_test.dart:98: Date date = new Date(year, month,
day, hours, minutes, seconds, 0, isUtc: true);
Long line.

[Anders Johnsen, 2012-09-04 07:03:13]

Thank you, landing.

https://chromiumcodereview.appspot.com/10909057/diff/1/runtime/bin/http_utils...
File runtime/bin/http_utils.dart (right):

https://chromiumcodereview.appspot.com/10909057/diff/1/runtime/bin/http_utils...
runtime/bin/http_utils.dart:253: throw new HttpException("Invalid HTTP date
$date");
On 2012/09/04 06:51:57, Søren Gjesse wrote:
> HTTP -> cookie

Done.

https://chromiumcodereview.appspot.com/10909057/diff/1/runtime/bin/http_utils...
runtime/bin/http_utils.dart:257: return position >= date.length;
On 2012/09/04 06:51:57, Søren Gjesse wrote:
> Shouldn't this be just ==?

In theory, yes. I'll change. If we crash without it would expose a bug - better
to actually show that then just ignore.

https://chromiumcodereview.appspot.com/10909057/diff/1/runtime/bin/http_utils...
runtime/bin/http_utils.dart:260: bool isDelimiter(String s) {
On 2012/09/04 06:51:57, Søren Gjesse wrote:
> How about passing the index here and use s.charCodeAt(index) instead of
creating
> single character substrings.

Keeping, as discussed off-list.

https://chromiumcodereview.appspot.com/10909057/diff/1/runtime/bin/http_utils...
runtime/bin/http_utils.dart:304: while (!isEnd() &&
isNonDelimiter(date[position])) {
On 2012/09/04 06:51:57, Søren Gjesse wrote:
> How about saving position before the loop, just incrementing position in the
> loop and making a substring after the loop?

Done.

https://chromiumcodereview.appspot.com/10909057/diff/1/runtime/bin/http_utils...
runtime/bin/http_utils.dart:317: if (token.length < 1) return null;
On 2012/09/04 06:51:57, Søren Gjesse wrote:
> Shouldn't this be a continue instead of return null?

That is a very good point! Changed.

https://chromiumcodereview.appspot.com/10909057/diff/1/tests/standalone/io/ht...
File tests/standalone/io/http_date_test.dart (right):

https://chromiumcodereview.appspot.com/10909057/diff/1/tests/standalone/io/ht...
tests/standalone/io/http_date_test.dart:98: Date date = new Date(year, month,
day, hours, minutes, seconds, 0, isUtc: true);
On 2012/09/04 06:51:57, Søren Gjesse wrote:
> Long line.

Done.

[Mads Ager (google), 2012-09-04 08:41:14]

DBC

This was pretty simple, thanks Anders.

https://chromiumcodereview.appspot.com/10909057/diff/5/runtime/bin/http_utils...
File runtime/bin/http_utils.dart (right):

https://chromiumcodereview.appspot.com/10909057/diff/5/runtime/bin/http_utils...
runtime/bin/http_utils.dart:122: List wkdays = ["Mon", "Tue", "Wed", "Thu",
"Fri", "Sat", "Sun"];
Not your code, but these could be:

const List wkdays = const ["Mon", ...];

https://chromiumcodereview.appspot.com/10909057/diff/5/runtime/bin/http_utils...
runtime/bin/http_utils.dart:247: List monthsLowerCase = ["jan", "feb", "mar",
"apr", "may", "jun",
Similarly, this could be explicitly marked as a const list.

https://chromiumcodereview.appspot.com/10909057/diff/5/runtime/bin/http_utils...
runtime/bin/http_utils.dart:329: monthStr === null || yearStr === null) error();
Since the condition alone covers more than one line I would find it more
readable to use braces:

if (this || that ||
    ... || theOtherThing) {
  error();
}

[Anders Johnsen, 2012-09-05 08:44:29]

See https://chromiumcodereview.appspot.com/10914093/

https://chromiumcodereview.appspot.com/10909057/diff/5/runtime/bin/http_utils...
File runtime/bin/http_utils.dart (right):

https://chromiumcodereview.appspot.com/10909057/diff/5/runtime/bin/http_utils...
runtime/bin/http_utils.dart:122: List wkdays = ["Mon", "Tue", "Wed", "Thu",
"Fri", "Sat", "Sun"];
On 2012/09/04 08:41:14, Mads Ager wrote:
> Not your code, but these could be:
> 
> const List wkdays = const ["Mon", ...];

Done.

https://chromiumcodereview.appspot.com/10909057/diff/5/runtime/bin/http_utils...
runtime/bin/http_utils.dart:247: List monthsLowerCase = ["jan", "feb", "mar",
"apr", "may", "jun",
On 2012/09/04 08:41:14, Mads Ager wrote:
> Similarly, this could be explicitly marked as a const list.

Done.

https://chromiumcodereview.appspot.com/10909057/diff/5/runtime/bin/http_utils...
runtime/bin/http_utils.dart:329: monthStr === null || yearStr === null) error();
On 2012/09/04 08:41:14, Mads Ager wrote:
> Since the condition alone covers more than one line I would find it more
> readable to use braces:
> 
> if (this || that ||
>     ... || theOtherThing) {
>   error();
> }

Done.