Opened 5 weeks ago

Closed 4 weeks ago

#31203 closed defect (fixed)

Fix Parse.byteCount in the case of unknown units

Reported by: dcf Owned by: dcf
Priority: Medium Milestone:
Component: Circumvention/Snowflake Version:
Severity: Normal Keywords:
Cc: arlolra, cohosh, phw, dcf Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Parse.byteCount is supposed to parse numbers followed by an optional units suffix, like "1500" or "1.5M". It behaves incorrectly in the case of an unrecognized suffix ("1.5X"), returning NaN instead of null.

The first part of the patch adds tests for Parse.byteCount and Params.getByteCount, which fail:

1) Parse byte count returns null for bad inputs
  Message:
    Expected NaN to be null.
  Stack:
    Error: Expected NaN to be null.
        at Object.<anonymous> (snowflake/proxy/test/bundle.spec.js:1849:37)
  ...

The second part of the patch fixes Parse.byteCount so it doesn't fail.

Child Tickets

Attachments (1)

bug31203.patch (5.5 KB) - added by dcf 5 weeks ago.

Download all attachments as: .zip

Change History (8)

Changed 5 weeks ago by dcf

Attachment: bug31203.patch added

comment:1 Changed 5 weeks ago by dcf

Status: assignedneeds_review

comment:2 Changed 5 weeks ago by arlolra

Status: needs_reviewmerge_ready

lgtm

comment:3 Changed 5 weeks ago by dcf

Resolution: fixed
Status: merge_readyclosed

comment:4 Changed 4 weeks ago by dcf

Resolution: fixed
Status: closedreopened

Reopening because of a test failure reported in comment:6:ticket:31126.

comment:5 Changed 4 weeks ago by dcf

Status: reopenedneeds_review
  • proxy/spec/util.spec.js

    From e67a65994319d05fbfd22f00973587c0696ca81d Mon Sep 17 00:00:00 2001
    From: David Fifield <david@bamsoftware.com>
    Date: Thu, 25 Jul 2019 14:32:26 -0600
    Subject: [PATCH] Fix tests for Params.getByteCount.
    
    They were relying on the Query.parse interface, which was removed
    separately.
    
    https://bugs.torproject.org/31126#comment:5
    ---
     proxy/spec/util.spec.js | 2 +-
     1 file changed, 1 insertion(+), 1 deletion(-)
    
    diff --git a/proxy/spec/util.spec.js b/proxy/spec/util.spec.js
    index f48365d..6eb5be4 100644
    a b describe('Params', function() { 
    238238
    239239    var DEFAULT = 77;
    240240    var getByteCount = function(query) {
    241       return Params.getByteCount(Query.parse(query), 'param', DEFAULT);
     241      return Params.getByteCount(new URLSearchParams(query), 'param', DEFAULT);
    242242    };
    243243
    244244    it('supports default values', function() {

comment:6 Changed 4 weeks ago by arlolra

Status: needs_reviewmerge_ready

Whoops, yup, merge conflict. Please get this in.

comment:7 Changed 4 weeks ago by dcf

Resolution: fixed
Status: merge_readyclosed
Note: See TracTickets for help on using tickets.