Read only archive ; use https://github.com/JacORB/JacORB/issues for new issues
Bug 904 - CDRInputStream.read_string should handle 0 string size gracefully
Summary: CDRInputStream.read_string should handle 0 string size gracefully
Status: RESOLVED FIXED
Alias: None
Product: JacORB
Classification: Unclassified
Component: ORB (show other bugs)
Version: 2.3.1
Hardware: All All
: P2 enhancement
Assignee: Nick Cross
URL:
Depends on:
Blocks:
 
Reported: 2011-06-24 20:17 UTC by Fabiano Ghisla
Modified: 2011-09-12 17:27 UTC (History)
1 user (show)

See Also:

Attachments
simple (but wrong) fix (626 bytes, patch)
2011-06-24 20:20 UTC, Fabiano Ghisla 		Details | Diff
trivial fix (677 bytes, patch)
2011-06-24 20:21 UTC, Fabiano Ghisla 		Details | Diff
sample implementation of jacorb.interop.lax_null_string_encoding property (2.64 KB, patch)
2011-06-27 23:04 UTC, Fabiano Ghisla 		Details | Diff
unit test showing the property at work. (1.84 KB, text/plain)
2011-06-27 23:10 UTC, Fabiano Ghisla 		Details
Show Obsolete (2) Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Fabiano Ghisla 2011-06-24 20:17:10 UTC 
Some ORBs incorrectly encode empty strings as having size 0.
The pragmatic way to handle this case would be to return an empty string. Instead, read_string() throws a marshal exception whenever the size parameter is less than 1, making it very hard to use JacORB to integrate with a misbehaving server ORBs.
Comment 1 Fabiano Ghisla 2011-06-24 20:20:16 UTC 
Created attachment 387 [details]
simple (but wrong) fix
Comment 2 Fabiano Ghisla 2011-06-24 20:21:55 UTC 
Created attachment 388 [details]
trivial fix
Comment 3 Nick Cross 2011-06-27 09:12:19 UTC 
This patch would unfortunately make JacORB non-compliant with the specification and may well break the unit tests. 

Which ORBs are non-compliant?

An alternative and more compatible design would be to implement a jacorb property that would enable this behaviour. It should be defaulted to off so that JacORB is compliant out-of-the-box. This would also require changes to etc/jacorb_properties.template, the PDF documentation, doc/REL_NOTES, and ideally a unit test should also be added.
Comment 4 Fabiano Ghisla 2011-06-27 23:04:23 UTC 
Created attachment 389 [details]
sample implementation of jacorb.interop.lax_null_string_encoding property
Comment 5 Fabiano Ghisla 2011-06-27 23:10:15 UTC 
Created attachment 390 [details]
unit test showing the property at work.
Comment 6 Fabiano Ghisla 2011-06-27 23:20:21 UTC 
According to the vendor documentation, the ORB showing the wrong string encoding behaviour is Visibroker.

I've implemented a (hopefully) proper fix following your suggestions. ProgrammerGuide documentation, release notes and jacorb_properties.template updated as per your request. Was unsure how to add the unit test, so I resorted in updating the existing regression testcase (to test the existing default behaviour), as well as creating a new one testing the actual behaviour when the flag is set.
Comment 7 Nick Cross 2011-06-28 08:59:51 UTC 
Thats great; thanks for your effort; I'll take a look at it.
Comment 8 Nick Cross 2011-09-12 17:27:31 UTC 
Bugzilla 904/JAC#639 Enable interop null string marshalling.
doc/REL_NOTES 	
doc/ProgrammingGuide/ProgrammingGuide.pdf
doc/ProgrammingGuide/Configuration/configuration.tex
etc/jacorb_properties.template 	
src/org/jacorb/orb/CDRInputStream.java
src/org/jacorb/orb/CDROutputStream.java
test/regression/src/org/jacorb/test/orb/CDRInputStreamNullStringTest.java
test/regression/src/org/jacorb/test/orb/CDRInputStreamTest.java