search

osgi / bugzilla-archive

Archive of OSGi Alliance Specification Bugzilla bugs. The Specification Bugzilla system was decommissioned with the move to GitHub. The issues in this repository are imported from the Specification Bugzilla system for archival purposes.
0 stars 1 forks source link

[PA Utility Classes] the empty string is valid TR-069 path #1803

Closed bjhargrave closed 13 years ago

bjhargrave commented 13 years ago

Original bug ID: BZ#1935 From: Evgeni Grigorov <e.grigorov@prosyst.com> Reported version: R4 V4.3

Duplicates: BZ#1954

bjhargrave commented 13 years ago

Comment author: Evgeni Grigorov <e.grigorov@prosyst.com>

TR069URI.isValidTR069Path("") returns true, but must be false

bjhargrave commented 13 years ago

Comment author: william.lupton@pace.com

TR069URI.isValidTR069Path("") returns true, but must be false

This path is valid for some RPCs (GPN, GPV, GPA, SPA), where it means "the top of the name hierarchy".

Presumably it translates to "./"?

bjhargrave commented 13 years ago

Comment author: Evgeni Grigorov <e.grigorov@prosyst.com>

TR069URI.isValidTR069Path("") returns true, but must be false

This path is valid for some RPCs (GPN, GPV, GPA, SPA), where it means "the top of the name hierarchy".

Presumably it translates to "./"? Good point, I'll check that.

What about TR069URI.isValidTR069Path(" ")? Currently, it returns false. If we apply the same rules as above, it should return true, right?

bjhargrave commented 13 years ago

Comment author: william.lupton@pace.com

What about TR069URI.isValidTR069Path(" ")? Currently, it returns false. If we apply the same rules as above, it should return true, right?

I'd say "no" (TR-069 mentions only the empty string "").

bjhargrave commented 13 years ago

Comment author: Evgeni Grigorov <e.grigorov@prosyst.com>

  1. TR069URI.getDmtUri("") returns "./", but should be "." Note that "./" is not valid DMT Uri, because trailing "/".
  2. TR069URI.getTR069Path(".") generates java.lang.StringIndexOutOfBoundsException: String index out of range: -1 at java.lang.String.substring(String.java:1937) at java.lang.String.substring(String.java:1904) at org.osgi.util.tr069.TR069URI.getTR069Path(TR069URI.java:71) ..... IMO, it should return an empty string, right?
bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

First of all, I had changed the javadoc from the RFC149.

Old: TR069URI.getDmtUri() will NOT validate the specified path. TR069URI.getTR069Path() will NOT validate the specified uri.

New: TR069URI.getDmtUri() will validate the specified path. TR069URI.getTR069Path() will validate the specified uri. If it is not either valid or absolute, throws IllegalArgumentException.

Secondly, I had added the following method in TR069URI, as info.dmtree.Uri has "isAbsoluteUri(uri)" and "isValidUri(uri)":

public static boolean isAbsoluteTR069Path(final String tr069Path):

Checks whether the specified path is an absolute TR-069 path. An absolute path contains the complete path, which starts with ".".

@ param uri the TR-069 path to be checked, must contain a valid TR-069 path @ return whether the specified path is absolute @ throws IllegalArgumentException if the specified path is invalid TR-069 path

Then,

1-1. TR069URI.isValidTR069Path(".") should return true. 1-2. TR069URI.isValidTR069Path("") should return true. 1-3. TR069URI.isValidTR069Path(" ") should return false. 1-4. TR069URI.isValidTR069Path("./") should return false.

2-1. TR069URI.isAbsoluteTR069Path(".") should return false. 2-2. TR069URI.isAbsoluteTR069Path("") should return true. 2-3. TR069URI.isAbsoluteTR069Path(" ") should throw IllegalArgumentException. 2-4. TR069URI.isAbsoluteTR069Path("./") should throw IllegalArgumentException.

3-1. TR069URI.getDmtUri(".") should throw IllegalArgumentException. 3-2. TR069URI.getDmtUri("") should return ".". 3-3. TR069URI.getDmtUri(" ") should throw IllegalArgumentException. 3-4. TR069URI.getDmtUri("./") should throw IllegalArgumentException.

4-1. TR069URI.getTR069Path(".") should return "". 4-2. TR069URI.getTR069Path("") should throw IllegalArgumentException. 4-3. TR069URI.getTR069Path(" ") should throw IllegalArgumentException. 4-4. TR069URI.getTR069Path("./") should throw IllegalArgumentException.

Is that OK ?

  1. TR069URI.isValidTR069Path(" ") should return false.
bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

Please ignore the last sentence. That should have been removed.

  1. TR069URI.isValidTR069Path(" ") should return false.
bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

Although I did not get the confirmation in comment BZ#5, I implemented the updated RI in Git Repo as written.

bjhargrave commented 13 years ago

Comment author: Evgeni Grigorov <e.grigorov@prosyst.com>

First of all, I had changed the javadoc from the RFC149.

Old: TR069URI.getDmtUri() will NOT validate the specified path. TR069URI.getTR069Path() will NOT validate the specified uri.

New: TR069URI.getDmtUri() will validate the specified path. TR069URI.getTR069Path() will validate the specified uri. If it is not either valid or absolute, throws IllegalArgumentException. It's an old discussion. REG decided that validation should not be a part of those get methods. As I can remember, the general reason is the performance. If PA wants to validate the path, it can call isValid..... Can you revert that modification? TR-069 PA Utility Classes TC doesn't abuse with that behavior.

Secondly, I had added the following method in TR069URI, as info.dmtree.Uri has "isAbsoluteUri(uri)" and "isValidUri(uri)":

public static boolean isAbsoluteTR069Path(final String tr069Path): TR-069 PA Utility Classes TC needs to cover that new method. Can you post a new issue?

Checks whether the specified path is an absolute TR-069 path. An absolute path contains the complete path, which starts with ".".

@ param uri the TR-069 path to be checked, must contain a valid TR-069 path @ return whether the specified path is absolute @ throws IllegalArgumentException if the specified path is invalid TR-069 path

Then,

1-1. TR069URI.isValidTR069Path(".") should return true. 1-2. TR069URI.isValidTR069Path("") should return true. 1-3. TR069URI.isValidTR069Path(" ") should return false. 1-4. TR069URI.isValidTR069Path("./") should return false.

2-1. TR069URI.isAbsoluteTR069Path(".") should return false. 2-2. TR069URI.isAbsoluteTR069Path("") should return true. 2-3. TR069URI.isAbsoluteTR069Path(" ") should throw IllegalArgumentException. 2-4. TR069URI.isAbsoluteTR069Path("./") should throw IllegalArgumentException.

3-1. TR069URI.getDmtUri(".") should throw IllegalArgumentException. 3-2. TR069URI.getDmtUri("") should return ".". 3-3. TR069URI.getDmtUri(" ") should throw IllegalArgumentException. 3-4. TR069URI.getDmtUri("./") should throw IllegalArgumentException.

4-1. TR069URI.getTR069Path(".") should return "". 4-2. TR069URI.getTR069Path("") should throw IllegalArgumentException. 4-3. TR069URI.getTR069Path(" ") should throw IllegalArgumentException. 4-4. TR069URI.getTR069Path("./") should throw IllegalArgumentException.

Is that OK ? Looks ok to me.

bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

First of all, I had changed the javadoc from the RFC149.

Old: TR069URI.getDmtUri() will NOT validate the specified path. TR069URI.getTR069Path() will NOT validate the specified uri.

New: TR069URI.getDmtUri() will validate the specified path. TR069URI.getTR069Path() will validate the specified uri. If it is not either valid or absolute, throws IllegalArgumentException. It's an old discussion. REG decided that validation should not be a part of those get methods. As I can remember, the general reason is the performance.

I remember the discussion. However, your following answer A and B are contradictory :

A)

If PA wants to validate the path, it can call isValid..... Can you revert that modification? TR-069 PA Utility Classes TC doesn't abuse with that behavior.

B)

3-3. TR069URI.getDmtUri(" ") should throw IllegalArgumentException. 3-4. TR069URI.getDmtUri("./") should throw IllegalArgumentException. 4-3. TR069URI.getTR069Path(" ") should throw IllegalArgumentException. 4-4. TR069URI.getTR069Path("./") should throw IllegalArgumentException. Is that OK ? Looks ok to me.

If getDmtUri() and getTR069Path() does NOT validate the argument, there is no way for the impl of those methods to recognize illegal argument exception for " " or "./".

If we follow "getDmtUri() and getTR069Path() does NOT validate the argument", I cannot define what kind of value to be returned for those:

3-3. TR069URI.getDmtUri(" ")
3-4. TR069URI.getDmtUri("./") 4-3. TR069URI.getTR069Path(" ") 4-4. TR069URI.getTR069Path("./")

Do you have any idea ?

bjhargrave commented 13 years ago

Comment author: Evgeni Grigorov <e.grigorov@prosyst.com>

First of all, I had changed the javadoc from the RFC149.

Old: TR069URI.getDmtUri() will NOT validate the specified path. TR069URI.getTR069Path() will NOT validate the specified uri.

New: TR069URI.getDmtUri() will validate the specified path. TR069URI.getTR069Path() will validate the specified uri. If it is not either valid or absolute, throws IllegalArgumentException. It's an old discussion. REG decided that validation should not be a part of those get methods. As I can remember, the general reason is the performance.

I remember the discussion. However, your following answer A and B are contradictory :

A)

If PA wants to validate the path, it can call isValid..... Can you revert that modification? TR-069 PA Utility Classes TC doesn't abuse with that behavior.

B)

3-3. TR069URI.getDmtUri(" ") should throw IllegalArgumentException. 3-4. TR069URI.getDmtUri("./") should throw IllegalArgumentException. 4-3. TR069URI.getTR069Path(" ") should throw IllegalArgumentException. 4-4. TR069URI.getTR069Path("./") should throw IllegalArgumentException. Is that OK ? Looks ok to me.

If getDmtUri() and getTR069Path() does NOT validate the argument, there is no way for the impl of those methods to recognize illegal argument exception for " " or "./".

If we follow "getDmtUri() and getTR069Path() does NOT validate the argument", I cannot define what kind of value to be returned for those:

3-3. TR069URI.getDmtUri(" ")
3-4. TR069URI.getDmtUri("./") 4-3. TR069URI.getTR069Path(" ") 4-4. TR069URI.getTR069Path("./")

Do you have any idea ? :) Ok, I'll clarify, it's fine to me in the context of no validation in "get" methods. The behavior of getDmtUri and getTR069Path is unpredictable for invalid path arguments. So, just remove 3-3, 3-4, 4-3 and 4-4.

bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

:) Ok, I'll clarify, it's fine to me in the context of no validation in "get" methods. The behavior of getDmtUri and getTR069Path is unpredictable for invalid path arguments. So, just remove 3-3, 3-4, 4-3 and 4-4.

I Got it. Summary:

a) getDmtUri() and getTR069Path() does NOT validate the argument. b) isAbsoluteTR069Path(".") validate the argument as Uri.isAbsoluteUri() does.

1-1. TR069URI.isValidTR069Path(".") should return true. 1-2. TR069URI.isValidTR069Path("") should return true. 1-3. TR069URI.isValidTR069Path(" ") should return false. 1-4. TR069URI.isValidTR069Path("./") should return false.

2-1. TR069URI.isAbsoluteTR069Path(".") should return false. 2-2. TR069URI.isAbsoluteTR069Path("") should return true. 2-3. TR069URI.isAbsoluteTR069Path(" ") should throw IllegalArgumentException. 2-4. TR069URI.isAbsoluteTR069Path("./") should throw IllegalArgumentException.

3-1. TR069URI.getDmtUri(".") should throw IllegalArgumentException. 3-2. TR069URI.getDmtUri("") should return ".". 3-3. TR069URI.getDmtUri(" ") should NOT throw IllegalArgumentException but return unpredictable String. 3-4. TR069URI.getDmtUri("./") should Not throw IllegalArgumentException but return unpredictable String. 4-1. TR069URI.getTR069Path(".") should return "". 4-2. TR069URI.getTR069Path("") should throw IllegalArgumentException. 4-3. TR069URI.getTR069Path(" ") should throw IllegalArgumentException but return unpredictable String. 4-4. TR069URI.getTR069Path("./") should throw IllegalArgumentException but return unpredictable String.

Is it correct (especially for b)?

bjhargrave commented 13 years ago

Comment author: Evgeni Grigorov <e.grigorov@prosyst.com>

Just a few confirmation comments and a question to William!

:) Ok, I'll clarify, it's fine to me in the context of no validation in "get" methods. The behavior of getDmtUri and getTR069Path is unpredictable for invalid path arguments. So, just remove 3-3, 3-4, 4-3 and 4-4.

I Got it. Summary:

a) getDmtUri() and getTR069Path() does NOT validate the argument. b) isAbsoluteTR069Path(".") validate the argument as Uri.isAbsoluteUri() does.

Yes, because the absolute path is a valid path.

1-1. TR069URI.isValidTR069Path(".") should return true.

William, is it ok?

1-2. TR069URI.isValidTR069Path("") should return true. 1-3. TR069URI.isValidTR069Path(" ") should return false. 1-4. TR069URI.isValidTR069Path("./") should return false.

2-1. TR069URI.isAbsoluteTR069Path(".") should return false. 2-2. TR069URI.isAbsoluteTR069Path("") should return true. 2-3. TR069URI.isAbsoluteTR069Path(" ") should throw IllegalArgumentException. 2-4. TR069URI.isAbsoluteTR069Path("./") should throw IllegalArgumentException.

3-1. TR069URI.getDmtUri(".") should throw IllegalArgumentException. The result should be unpredictable, because "." is not absolute. 3-2. TR069URI.getDmtUri("") should return ".". 3-3. TR069URI.getDmtUri(" ") should NOT throw IllegalArgumentException but return unpredictable String. The result is unpredictable. That means that it's up to the implementation to decide between strange string result or an exception. Your choice is ok to me. 3-4. TR069URI.getDmtUri("./") should Not throw IllegalArgumentException but return unpredictable String. 4-1. TR069URI.getTR069Path(".") should return "". 4-2. TR069URI.getTR069Path("") should throw IllegalArgumentException. The result should be unpredictable. 4-3. TR069URI.getTR069Path(" ") should throw IllegalArgumentException but return unpredictable String. 4-4. TR069URI.getTR069Path("./") should throw IllegalArgumentException but return unpredictable String.

Is it correct (especially for b)?

bjhargrave commented 13 years ago

Comment author: Evgeni Grigorov <e.grigorov@prosyst.com>

[06.04.2011] Conference Call William confirmed that TR069URI.isValidTR069Path(".") should return true.

bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

Close this bug because the discussion will be continued in Bug#1954.

This bug has been marked as a duplicate of bug BZ#1954