Merge pull request #21 from OpenNMS/jira/NMS-9578

NMS-9578: URL-encode FIQL values
OpenNMS · Aug 30, 2017 · dc78a13 · dc78a13
2 parents 2f360fd + 9a335e8
commit dc78a13
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 17 deletions.
diff --git a/src/dao/V2FilterProcessor.ts b/src/dao/V2FilterProcessor.ts
@@ -26,7 +26,13 @@ export class V2FilterProcessor implements IFilterProcessor {
    *  This must be explicitly set as the restriction value when using
    *  either the NULL or NOTNULL comparators on date fields.
    */
-  public static NULL_DATE = '1970-01-01T00:00:00.000-0000';
+  public static NULL_DATE = '1970-01-01T00:00:00.000+0000';
+
+  /**
+   * pre-encoded to avoid running `encodeURIComponent` every time we deal with a null date
+   * @hidden
+   */
+  private static NULL_DATE_ENCODED = encodeURIComponent(V2FilterProcessor.NULL_DATE);
 
   /** The accessor for Properties */
   private searchPropertyAccessor: ISearchPropertyAccessor;
@@ -84,16 +90,16 @@ export class V2FilterProcessor implements IFilterProcessor {
     switch (restriction.comparator) {
       case Comparators.NULL:
       case Comparators.NOTNULL:
-          return restriction.value === undefined ? V2FilterProcessor.NULL_VALUE : restriction.value;
+          return restriction.value === undefined ? V2FilterProcessor.NULL_VALUE : encodeURIComponent(restriction.value);
       default:
           if (restriction.value === 'null' || restriction.value === void 0) {
               const property = this.searchPropertyAccessor.getProperty(restriction.attribute);
               if (property && property.type === SearchPropertyTypes.TIMESTAMP) {
-                  return V2FilterProcessor.NULL_DATE;
+                  return V2FilterProcessor.NULL_DATE_ENCODED;
               }
               return V2FilterProcessor.NULL_VALUE;
           }
-          return this.applyDateConversion(restriction.value);
+          return encodeURIComponent(this.applyDateConversion(restriction.value));
     }
   }
 

diff --git a/src/internal/Util.ts b/src/internal/Util.ts
@@ -64,7 +64,7 @@ export class Util {
   public static toDateString(date: Date|Moment|number) {
     const ret = Util.toMoment(date);
     if (ret) {
-      return ret.utc().format(dateFormat).replace('+0000', '-0000');
+      return ret.utc().format(dateFormat);
     } else {
       return undefined;
     }

diff --git a/test/dao/V1FilterProcessor.spec.ts b/test/dao/V1FilterProcessor.spec.ts
@@ -77,12 +77,12 @@ describe('V1FilterProcessor', () => {
       proc.getParameters(filter);
     }).toThrow(OnmsError);
   });
-  it('alarm filter: lastEventTime=1976-04-14T00:00:00.000-0000', () => {
+  it('alarm filter: lastEventTime=1976-04-14T00:00:00.000+0000', () => {
     const filter = new Filter();
     filter.withOrRestriction(new Restriction('lastEventTime', Comparators.EQ, new Date(198288000000)));
     const proc = new V1FilterProcessor();
     expect(proc.getParameters(filter)).toMatchObject({
-      lastEventTime: '1976-04-14T00:00:00.000-0000'
+      lastEventTime: '1976-04-14T00:00:00.000+0000'
     });
   });
 });
diff --git a/test/dao/V2FilterProcessor.spec.ts b/test/dao/V2FilterProcessor.spec.ts
@@ -47,8 +47,8 @@ describe('V2FilterProcessor', () => {
   it('alarm filter: id=notnull OR ackTime=null', () => {
     const filter = new Filter();
     filter.withOrRestriction(new Restriction('id', Comparators.NOTNULL));
-    filter.withOrRestriction(new Restriction('ackTime', Comparators.NULL, V2FilterProcessor.NULL_DATE));
-    expect(toSearch(filter)).toEqual('id!=\u0000,ackTime==1970-01-01T00:00:00.000-0000');
+    filter.withOrRestriction(new Restriction('ackTime', Comparators.NULL, '1970-01-01T00:00:00.000+0000'));
+    expect(toSearch(filter)).toEqual('id!=\u0000,ackTime==1970-01-01T00%3A00%3A00.000%2B0000');
   });
   it('alarm filter: id=notnull OR severity="MINOR"', () => {
     const filter = new Filter();
@@ -95,10 +95,10 @@ describe('V2FilterProcessor', () => {
         );
     expect(toSearch(filter)).toEqual('id!=0;(severity==5,uei==*somethingWentWrong)');
   });
-  it('alarm filter: lastEventTime=1976-04-14T00:00:00.000-0000', () => {
+  it('alarm filter: lastEventTime=1976-04-14T00:00:00.000+0000', () => {
     const filter = new Filter();
     filter.withAndRestriction(new Restriction('lastEventTime', Comparators.EQ, new Date(198288000000)));
-    expect(toSearch(filter)).toEqual('lastEventTime==1976-04-14T00:00:00.000-0000');
+    expect(toSearch(filter)).toEqual('lastEventTime==1976-04-14T00%3A00%3A00.000%2B0000');
   });
   it('alarm filter: verify null replacement for EQ and NE comparators', () => {
       // the filter does not make any sense, but is there to verify that null replacement works correctly
@@ -112,8 +112,8 @@ describe('V2FilterProcessor', () => {
               { id: 'alarmAckTime', type: SearchPropertyTypes.TIMESTAMP } as SearchProperty,
           ]));
       expect(toSearch(filter, proc)).toEqual(
-          'alarmAckTime==1970-01-01T00:00:00.000-0000'
-          + ';alarmAckTime!=1970-01-01T00:00:00.000-0000'
+          'alarmAckTime==1970-01-01T00%3A00%3A00.000%2B0000'
+          + ';alarmAckTime!=1970-01-01T00%3A00%3A00.000%2B0000'
           + ';id==\u0000;id!=\u0000');
   });
 });
diff --git a/test/internal/Util.spec.ts b/test/internal/Util.spec.ts
@@ -10,7 +10,7 @@ import {Moment} from 'moment';
 const moment = require('moment');
 
 /** @hidden */
-const ARBITRARY_STRING = '2017-08-08T12:29:56.000-0000';
+const ARBITRARY_STRING = '2017-08-08T12:29:56.000+0000';
 
 /** @hidden */
 const ARBITRARY_EPOCH = 1502195396000;
@@ -95,15 +95,15 @@ describe('Util.toDateString()', () => {
     expect(Util.toDateString(null)).toBeUndefined();
   });
   it('moment(0)', () => {
-    expect(Util.toDateString(moment(0))).toEqual('1970-01-01T00:00:00.000-0000');
+    expect(Util.toDateString(moment(0))).toEqual('1970-01-01T00:00:00.000+0000');
   });
   it('0', () => {
-    expect(Util.toDateString(0)).toEqual('1970-01-01T00:00:00.000-0000');
+    expect(Util.toDateString(0)).toEqual('1970-01-01T00:00:00.000+0000');
   });
   it('new Date()', () => {
     expect(Util.toDateString(new Date(ARBITRARY_EPOCH))).toEqual(ARBITRARY_STRING);
   });
   it('new Date(0)', () => {
-    expect(Util.toDateString(new Date(0))).toEqual('1970-01-01T00:00:00.000-0000');
+    expect(Util.toDateString(new Date(0))).toEqual('1970-01-01T00:00:00.000+0000');
   });
 });