libyang icon indicating copy to clipboard operation
libyang copied to clipboard

Invalid date-and-time transform to a valid value, is it expected by designed?

Open lyzliyuzhi91 opened this issue 2 years ago • 11 comments

for example, 2023-02-29T14:21:23Z will be transform into 2023-03-01T22:21:23+08:00 , but there is no 02-29 in 2023 in real world. Is it expected ? version 2.1.30. Thanks

lyzliyuzhi91 avatar Aug 26 '23 02:08 lyzliyuzhi91

As for the date-and-time transformation, could libyang support an option or MACRO to disable the transformation. I know that the transformation conform to RFC. But I encounter such circumstance: After I reboot my embedded device, and then the system has not timezone info during the initiating stage. So I don't want to have any transformation.

And I think it may be not suitable to transform the value in some situation, for example, for an database program, it may just want to store the original input ? But once I call lyd_print_ apis, I will get an transformed value ?

And if a key of list is with date-and-time type, I think keys with value of 2022-02-01T00:00:00Z and 2022-02-01T08:00:00+08:00 should represent two different list entry?

lyzliyuzhi91 avatar Aug 26 '23 04:08 lyzliyuzhi91

After I reboot my embedded device, and then the system has not timezone info during the initiating stage.

That looks like something that's easily fixed. If you have some persistent storage location for YANG data (sysrepo's /etc/sysrepo, essentially), surely you can store a simple string with timezone information as well. Fixing that sounds better than adding a flag "don't do as the RFC says and instead violate that standard, please".

And if a key of list is with date-and-time type, I think keys with value of 2022-02-01T00:00:00Z and 2022-02-01T08:00:00+08:00 should represent two different list entry?

The standard is very clear on this matter (emphasis mine):

Note that since all leaf values in the data tree are conceptually stored in their canonical form (see Section 9.1), any XPath comparisons are done on the canonical value.

jktjkt avatar Aug 27 '23 21:08 jktjkt

If I set a configuartion

module test {
	namespace "urn:test";
	prefix test;
	
	import ietf-yang-types {
		prefix yang;
	}	
	
	container con {
		list time-range {
			key "start-time";
			leaf start-time {
				type yang:date-and-time;
			}
			
			leaf else {
				type string;
			}
		}
	}
}
<edit-config>
	<target>
		<running/>
	</target>	
	<config>
		<con xmlns="urn:test">
			<time-range>
				<start-time>2022-02-01T00:00:00Z</start-time>
				<else>something</else>
			</time-range>
		</con>	
	</config>
</edit-config>

And If the netconf server change the start-time value form, and if I send a get rpc with filter only with the container:

<get xmlns="urn:ietf:params:xml:ns:netconf:base:1.0">
	<filter type="subtree">
		<con xmlns="urn:test">
		</con>
	</filter>                                                                                         
</get>

then it should reply

<data>
	<con xmlns="urn:test">
		<time-range>
			<start-time>2022-02-01T08:00:00+08:00</start-time>
			<else>something</else>
		</time-range>
	</con>
</data>

And then if I send a get rpc with filter deep to the list time-range and the origin value in edit-config 2022-02-01T00:00:00Z, so do you think it is better to reply data with a different key value

<data>
	<con xmlns="urn:test">
		<time-range>
			<start-time>2022-02-01T08:00:00+08:00</start-time>
			<else>something</else>
		</time-range>
	</con>
</data>

than repy nothing in such design ?

<data/>

lyzliyuzhi91 avatar Aug 28 '23 03:08 lyzliyuzhi91

The standard is very clear on this matter (emphasis mine):

Note that since all leaf values in the data tree are conceptually stored in their canonical form (see Section 9.1), any XPath comparisons are done on the canonical value.

But I think Section 9.1 in RFC7950 only constrain to built-in type, because the section 9 is Built-In Types, and so I think it should to expand to other derived type.

lyzliyuzhi91 avatar Aug 28 '23 09:08 lyzliyuzhi91

The standard is very clear on this matter (emphasis mine):

Note that since all leaf values in the data tree are conceptually stored in their canonical form (see Section 9.1), any XPath comparisons are done on the canonical value.

But I think Section 9.1 in RFC7950 only constrain to built-in type, because the section 9 is Built-In Types, and so I think it should to expand to other derived type.

That's not what I said; I pointed to section 7.5.3 which explains a core language feature (must), and which also clarifies how the canonical form is important, and what it is used for. As you correctly point out, that section also says that for the built-in types, the canonical form is defined in section 9. However, for the yang:date-and-time you have to consult RFC 6991, and again, the canonization is mandatory.

It seems to me that you're proposing a change in a library to violate a RFC requirement "just" to address a shortcoming of your HW platform. I don't think that's gonna happen, to be honest, especially when the fix is trivial ("remember the TZ across reboots") and technically possible ("you already remember the YANG data; why cannot you remember one more string?").

jktjkt avatar Aug 28 '23 09:08 jktjkt

for example, 2023-02-29T14:21:23Z will be transform into 2023-03-01T22:21:23+08:00 , but there is no 02-29 in 2023 in real world. Is it >expected ?

And is it expected ?

lyzliyuzhi91 avatar Aug 28 '23 10:08 lyzliyuzhi91

https://github.com/CESNET/libyang/issues/2072

jktjkt avatar Aug 28 '23 10:08 jktjkt

If module is fixed like this:

module test {
	namespace "urn:test";
	prefix test;
	
	import ietf-yang-types {
		prefix yang;
	}	
	
	container con {
		list time-range {
			key "start-time";
			leaf start-time {
                              type yang:date-and-time {
                                  pattern '\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(\.\d+)?'
                                            + '(Z)';  // such pattern modification is allowed in RFC7950, which make date-and-time value is valid in Z-suffix form
                              }
			}
			
			leaf else {
				type string;
			}
		}
	}
}

And then if client send get rpc to query data:

<get xmlns="urn:ietf:params:xml:ns:netconf:base:1.0">
	<filter type="subtree">
		<con xmlns="urn:test">
		      <time-range>
			      <start-time>2022-02-01T08:00:00Z</start-time>
		      </time-range>                
		</con>
	</filter>                                                                                         
</get>

If the server use such canonization , the server will reply with invalid value

<data>
	<con xmlns="urn:test">
		<time-range>
			<start-time>2022-02-01T08:00:00+08:00</start-time> <!-- such canonization value violates pattern in module -->
			<else>something</else>
		</time-range>
	</con>
</data>

where start-time's value violates the pattern written in .yang module.

lyzliyuzhi91 avatar Aug 29 '23 06:08 lyzliyuzhi91

You're changing the regular expression pattern for a standard data type, and it seems to me that you're hoping that libyang will "pick it up" and produce custom output. That's not going to work. If you changed the pattern to use, say, two-digit years, how could the code have picked up the intention? Anyway, libyang uses a special, "hardcoded" implementation for some of these well-known data types, including yang:date-and-time, and one of the reasons for that are the canonicalization requirements.

jktjkt avatar Aug 29 '23 07:08 jktjkt

So I just don't think it is a good idea to canonicalize the non-built-in type in everywhere, because something can derive from the non-built-in type and change its pattern, and this is allowed in the rfc. And rfc7950 only restrict canonicalization to built-in type in section 9. I think libyang could support all these canonicalization by default but meanwhile let the developor to dicide whether to forbid the non-built-in type canonicalization due to the uncommon corner case.

lyzliyuzhi91 avatar Aug 29 '23 07:08 lyzliyuzhi91

I believe you have restricted the value in an unsupported way in your example. You can only further restrict the value space but you have changed it (to an invalid output, the Z form is never printed). libyang is not able to verify this for patterns so, unfortunately, it will allow you to load such a module. If you really require this feature, write a new typedef instead and implement a plugin for it. That way it can behave exactly as you want it to.

michalvasko avatar Sep 04 '23 07:09 michalvasko