[Bug 64791] New: getWriteAccess() call in InternalWorkbook always creates a new record (for in-memory workbooks)

classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug 64791] New: getWriteAccess() call in InternalWorkbook always creates a new record (for in-memory workbooks)

Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64791

            Bug ID: 64791
           Summary: getWriteAccess() call in InternalWorkbook always
                    creates a new record (for in-memory workbooks)
           Product: POI
           Version: unspecified
          Hardware: PC
            Status: NEW
          Severity: normal
          Priority: P2
         Component: HSSF
          Assignee: [hidden email]
          Reporter: [hidden email]
  Target Milestone: ---

The "getWriteAccess()" method in InternalWorkbook
(org.apache.poi.hssf.model.InternalWorkbook) checks if the "writeAccess" member
is non-null, and if not calls "createWriteAccess" and sets the "writeAccess"
member.

However, "createWorkbook()" (that is, the in-memory version) just calls
"createWriteAccess" which does NOT set the "writeAccess" member, meaning that a
subsequent call to "getWriteAccess" creates a second record. This same paradigm
/ bug may apply to other records as well (I haven't checked).

Anyway, I think the best way to address this is to make this change to
"InternalWorkbook.createWorkbook()" (Apache POI 4.1.2 code):
@@ -336,7 +336,7 @@
          records.add(new InterfaceHdrRecord(CODEPAGE));
          records.add(createMMS());
          records.add(InterfaceEndRecord.instance);
-         records.add(createWriteAccess());
+         getWriteAccess();
          records.add(createCodepage());
          records.add(createDSF());
          records.add(createTabId());

which will correctly set "writeAccess" and still put the record in the correct
position in the BIFF stream.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64791] getWriteAccess() call in InternalWorkbook always creates a new record (for in-memory workbooks)

Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64791

[hidden email] changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 OS|                            |All
                 CC|                            |[hidden email]

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64791] getWriteAccess() call in InternalWorkbook always creates a new record (for in-memory workbooks)

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64791

--- Comment #1 from [hidden email] ---
Real patch file and unit tests are forthcoming.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64791] getWriteAccess() call in InternalWorkbook always creates a new record (for in-memory workbooks)

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64791

--- Comment #2 from [hidden email] ---
Created attachment 37480
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=37480&action=edit
XLS file with two different writeAccess records

XLS file was created with "new HSSFWorkbook", etc. then did:
HSSFWorkbook wb = new HSSFWorkbook();
...
InternalWorkbook internalWorkbook = wb.getInternalWorkbook();
WriteAccessRecord writeAccess = internalWorkbook.getWriteAccess();
writeAccess.setUsername("gvtrwhit");

then write out the bytes with "wb.write(...)"

So, simply the call to "getWriteAccess()" created the second record (which then
had its username field set).

BiffViewer shows this:
Offset=0x00000106(262) recno=5 sid=0x00E2 size=0x0000(0)
[INTERFACEEND/]

Offset=0x0000010A(266) recno=6 sid=0x005C size=0x0070(112)
[WRITEACCESS]
    .name = gvtrwhit
[/WRITEACCESS]

Offset=0x0000017E(382) recno=7 sid=0x005C size=0x0070(112)
[WRITEACCESS]
    .name = gvtsstag
[/WRITEACCESS]

Offset=0x000001F2(498) recno=8 sid=0x0042 size=0x0002(2)
[CODEPAGE]
    .codepage        = 4b0
[/CODEPAGE]

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64791] getWriteAccess() call in InternalWorkbook always creates a new record (for in-memory workbooks)

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64791

--- Comment #3 from [hidden email] ---
Created attachment 37481
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=37481&action=edit
Patch file with changes to InternalWorkbook and TestWorkbook (unit test)

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64791] getWriteAccess() call in InternalWorkbook always creates a new record (for in-memory workbooks)

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64791

--- Comment #4 from [hidden email] ---
Created attachment 37482
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=37482&action=edit
Results of "ant test-main" before the change to InternalWorkbook

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64791] getWriteAccess() call in InternalWorkbook always creates a new record (for in-memory workbooks)

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64791

--- Comment #5 from [hidden email] ---
Created attachment 37483
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=37483&action=edit
Results of "ant test-main" after the InternalWorkbook change.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64791] getWriteAccess() call in InternalWorkbook always creates a new record (for in-memory workbooks)

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64791

--- Comment #6 from [hidden email] ---
Patch file created with "ant -f patch.xml"

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64791] [PATCH] getWriteAccess() call in InternalWorkbook always creates a new record (for in-memory workbooks)

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64791

[hidden email] changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|getWriteAccess() call in    |[PATCH] getWriteAccess()
                   |InternalWorkbook always     |call in InternalWorkbook
                   |creates a new record (for   |always creates a new record
                   |in-memory workbooks)        |(for in-memory workbooks)

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64791] [PATCH] getWriteAccess() call in InternalWorkbook always creates a new record (for in-memory workbooks)

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64791

Dominik Stadler <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |PatchAvailable

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64791] [PATCH] getWriteAccess() call in InternalWorkbook always creates a new record (for in-memory workbooks)

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64791

Dominik Stadler <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #37481|text/plain                  |application/gzip
          mime type|                            |
  Attachment #37481|1                           |0
           is patch|                            |

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64791] [PATCH] getWriteAccess() call in InternalWorkbook always creates a new record (for in-memory workbooks)

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64791

Dominik Stadler <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #7 from Dominik Stadler <[hidden email]> ---
Applied via r1882829, thanks for the patch, test and detailed explanation!

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64791] [PATCH] getWriteAccess() call in InternalWorkbook always creates a new record (for in-memory workbooks)

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64791

--- Comment #8 from [hidden email] ---
(In reply to Dominik Stadler from comment #7)
> Applied via r1882829, thanks for the patch, test and detailed explanation!

You're welcome. Just as a curiosity, do you guys ever backport fixes? I mean
are you considering a 4.1.3 at any time? So, would there be any need to merge
this change anywhere else besides trunk?

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64791] [PATCH] getWriteAccess() call in InternalWorkbook always creates a new record (for in-memory workbooks)

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64791

--- Comment #9 from [hidden email] ---
(In reply to Dominik Stadler from comment #7)
> Applied via r1882829, thanks for the patch, test and detailed explanation!

You're welcome. Just as a curiosity, do you guys ever backport fixes? I mean
are you considering a 4.1.3 at any time? So, would there be any need to merge
this change anywhere else besides trunk?

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64791] [PATCH] getWriteAccess() call in InternalWorkbook always creates a new record (for in-memory workbooks)

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64791

--- Comment #10 from PJ Fanning <[hidden email]> ---
We don't typically release patches for old code lines. We would probably only
consider it if there was a major bug or security issues.
POI 5.0.0 should be released by year end.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]