Negative duration

Issue #20 resolved
Ivan Kanakarakis created an issue

Hello, I selected this library to be used as part of pysaml2 - the SAML2 implementation in python.

The SAML2 dateTime datatype is based on XML Schema xs:dateTime datatype, which in turn is based on ISO 8601. One of the differences is that a duration is allowed to be negative.

An optional preceding minus sign ('-') is allowed, to indicate a negative duration. If the sign is omitted a positive duration is indicated. [0]

[...]

An optional minus sign is allowed immediately preceding, without a space, the lexical representations for duration, dateTime, date, gYearMonth, gYear. [1]

While this is not defined by ISO 8601, I see that negative durations are supported when defined within the period, for example PT-1S. Many libraries choose to support this format.

If this is not working by mistake, I would propose to also support the format that xml schema defines, which is an ISO 8601 duration format that starts with a negative sign, for example -P2Y. The python timedelta allows for such representation, and in fact

timedelta(seconds=-1) == -timedelta(seconds=1) == timedelta(seconds=1)*-1

This enables the implementation to be just a wrapper to the current behaviour.

period = '-PT1S'
is_negative = period[0] is '-'
sign = -1 if is_negative else 1
result = sign * parse_duration(period[is_negative:])

What do you think?

[0] https://www.w3.org/TR/xmlschema-2/#duration

[1] https://www.w3.org/TR/xmlschema-2/#signallowed

Comments (11)

  1. Brandon Nielsen repo owner

    I would consider the current behavior of parsing negative durations such as PT-1S a mistake. Clause 2.1.6 of ISO 8601 specifically calls out durations as "non-negative quantity attributed to a time interval".

    Are both PT-1S and -PT1S SAML2 compliant? My reading is that only the latter (-PT1S) is a legitimate negative duration under SAML2.

    Would you be hindered by my correcting the current behavior to throw an exception when parsing negative durations? To avoid me having the pour through the SAML2 spec, are there other wrappers that would be required to support SAML2?

  2. Ivan Kanakarakis reporter

    That is right:

    • ISO 8601 does not allow negative datetimes (thought, many libraries do support them)
    • SAML2 (through the xml schema spec) allows only the following negative form: -P<duration-specifiers>, as you pointed; where the sign is the first character (I will call that "global negative duration" and the other forms "inner negative duration")

    I proposed to make the "global" negative duration valid, as the other/inner negative duration forms were already valid. As you said, this is by mistake. If you prefer to be strict, and not support any form of negative duration, I understand the decision and it is fine :) The wrapper I proposed to support global negative duration, is really really simple and composes well with the current parse_duration function - I am already using it that way.

    I do not mind if inner negative durations are supported or not. Following the SAML2 spec strictly, they should not be allowed, but -along other things- implementations have relaxed these rules. If the inner negative durations are supported, it will be considered a bonus and aligned with the robustness principle.

    Regarding SAML2 and durations, AFAIK there is nothing more needed that this library does not offer, apart from the global negative duration format.

  3. Brandon Nielsen repo owner

    I try to be strict where possible, otherwise things tend to break for people if you become more strict later.

    In looking into it a bit more, I don't think you can call the current handling of negatives in durations anything but broken, for example:

    >>> aniso8601.parse_duration('PT-1H-10S')
    datetime.timedelta(-1, 82790)
    

    In future releases, expect the parsing of durations to become more strict with inner negative durations no longer being supported.

    Thank you for bringing this to my attention!

  4. Ivan Kanakarakis reporter
    >>> aniso8601.parse_duration('PT-1H-10S')
    datetime.timedelta(-1, 82790)
    

    This means that when you add this delta to some other delta or datetime object, that object will move backwords in time by 1day (-1) and then forwards by 82790 seconds. 1day is (60 * 60 * 24 =) 86400 seconds. 82790 means (86400 - 82790 =) 3610 seconds less than 1 day. 3610 is (3600 + 10 = 60 * 60 + 10) 1hour and 10seconds. So, to rephrase the initial sentence, this delta means that when it is added to a delta or datetime object it will move that object backwards in time by 1hour and 10seconds; which is the duration you want PT-1H-10S.

    In [1]: import aniso8601
    
    In [2]: t = aniso8601.parse_datetime('2018-12-12T01:00:10Z')
    
    In [3]: d = aniso8601.parse_duration('PT-1H-10S')
    
    In [4]: r = t + d
    
    In [5]: r.isoformat()
    Out[5]: '2018-12-12T00:00:00+00:00'
    

    t went from 01 to 00 hours, and from 10 to 00 seconds.

  5. Brandon Nielsen repo owner

    I agree the result is correct, but one negative in an inner duration is already against spec, accepting even more than one is just completely wrong as per the spec, especially given the results are not necessarily intuitive.

    For the sake of sanity, I'm still heavily in favor of not supporting negative durations in any form. Especially since its so easy to wrap parse_duration to support them if required.

  6. Ivan Kanakarakis reporter

    I just wanted to point out that the result is correct. The result is surely not intuitive; I went through the same exercise when I first came by it. It is fine if negative durations are not supported; it is probably the correct thing to do, considering the ISO 8601 standard and the goal of this library.

  7. Brandon Nielsen repo owner

    In the isobuilder branch, which will eventually become 4.0.0, negative durations throw a NegativeDurationError. Any negative duration support will have to implemented externally.

  8. Log in to comment