krakenjs / spud

A content store parser, reading a java .properties-like format
Other
14 stars 9 forks source link

with node4.2.3, if properties likes{object}=something, {object}.{property}=something exists, the parse.js throws exception. #25

Open XueliYue opened 8 years ago

XueliYue commented 8 years ago

I looked into the code of parse.js file.

In line 83, when executing tail[prop] = value, it may throws an exception if the tail[prop] is a string.

could someone please add one if-statement, in order to avoid this exception, the code like:

if(typeof tail === 'object') {
tail[prop] = value; //current code }

Thanks,

grawk commented 8 years ago

Can you provide a detailed use case where this exception would be thrown?

XueliYue commented 8 years ago

for example, here is properties file which includes two items as followings: CHAT_ENV=TEST CHAT_ENV.LOCATION=US

in node version 0.10, it seems work fine, the only console is the type of data.CHAT_ENV.LOCATION is function but in node version 4.2.3 , this line will throw a exception. because the tail = 'TEST, whose type is string , not a object.

kumarrishav commented 8 years ago

Yeah, facing same issue. hello.world = "hi" hello.world.universe = "bye"

It will throw error: Fatal error - cannot read universe property of hi . version: Node 4.5.0

grawk commented 8 years ago

The properties file is invalid in this case. The proposed solution would allow spud to silently skip over key/value pairs you thought you were defining.

The error message is currently pretty good as well. It is not difficult to infer the cause based on the fact that the error message includes the KVP strings where the problem originates.

The change I could envision making would be to throw an error here that's more specific: "Invalid key-value-pair detected. Don't assign a child key to a key which already defines a value."

Given the current default behavior is somewhat desirable but not perfect, this does not look like a high priority to me. But a PR would be welcome.

XueliYue commented 7 years ago

it does not matter with the error message, it break the left process. that is the reason i opened an pull request to silently skip it.