poi-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bugzi...@apache.org
Subject DO NOT REPLY [Bug 52661] An incomplete fix for the NPE bug in HyperlinkRecord.java
Date Wed, 15 Feb 2012 08:39:30 GMT
https://issues.apache.org/bugzilla/show_bug.cgi?id=52661

Yegor Kozlov <yegor@dinom.ru> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |NEEDINFO

--- Comment #3 from Yegor Kozlov <yegor@dinom.ru> 2012-02-15 08:39:30 UTC ---
I would say that toString() must correspond to serialize(), not the other way
around.

The code in HyperlinkRecord.serialize is correct. If a link is URL and not a
UNC path then the moniker field MUST be set. If it is null then you are
constructing wrong data.

HyperlinkRecord.toString() uses slightly different logic and prints the moniker
if it is not null and hyperlink type is URL (UNC path option is not tested
here). This is not quite correct but acceptable because toString() is used
mainly for debugging purposes. 

In any case, I'm not going to change it without a unit test that demonstrates
the NPE issue. If you are able to construct one, please upload and re-open this
ticket.

Yegor 

(In reply to comment #2)
> The text diff of the revision 720997 is as bellows: 
> --- poi/trunk/src/java/org/apache/poi/hssf/record/HyperlinkRecord.java   
> 2008/11/26 22:00:07    720996
> +++ poi/trunk/src/java/org/apache/poi/hssf/record/HyperlinkRecord.java   
> 2008/11/26 22:00:29    720997
> @@ -628,7 +628,7 @@
>          if ((_linkOpts & HLINK_TARGET_FRAME) != 0) {
>              buffer.append("    .targetFrame=
> ").append(getTargetFrame()).append("\n");
>          }
> -        if((_linkOpts & HLINK_URL) != 0) {
> +        if((_linkOpts & HLINK_URL) != 0 && _moniker != null) {
>              buffer.append("    .moniker   =
> ").append(_moniker.formatAsString()).append("\n");
>          }
>          if ((_linkOpts & HLINK_PLACE) != 0) {
> 
> we can see that only checking "(_linkOpts & HLINK_URL) != 0" can not guarantee
> that " _moniker != null". 
> 
> If the revision 720997 is a right fix, then I think the Line 559 of the method
> "serialize" should also be fixed this way. 
> 
> I think it will be more reasonable to treat them in a same way.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


Mime
View raw message