What is the use case for needing serialisation?
It's a lot of effort to maintain a serialisable class, and it opens
the class to deserialisation attacks.
On Sun, 19 Jan 2020 at 12:39, Alex Herbert <alex.d.herb...@gmail.com> wrote:
>
> Hi Gary,
>
> I raised a few niggles a while back with CSV and the discussion did not
> receive a response on how to proceed.
>
> There is the major bug CSV-248 where the CSVRecord is not Serializable [1].
> This requires a decision on what to do to fix it. This bug is still present
> in 1.8 RC1 as found by FindBugs [2].
>
> From what I can see the CSVRecord maintains a reference to the CSVParser.
> This chain of objects maintained in memory is not serializable and leads back
> to the original input Reader.
>
> I can see from the JApiCmp report that the serial version id was changed for
> CSVRecord this release so there is still an intention to support
> serialization. So this should be a blocker.
>
> I could not find a serialisation test in the unit tests for CSVRecord. This
> quick test added to CSVRecordTest fails:
>
>
> @Test
> public void testSerialization() throws IOException {
> CSVRecord shortRec;
> try (final CSVParser parser = CSVParser.parse("a,b",
> CSVFormat.newFormat(','))) {
> shortRec = parser.iterator().next();
> }
> final ByteArrayOutputStream out = new ByteArrayOutputStream();
> try (ObjectOutputStream oos = new ObjectOutputStream(out)) {
> oos.writeObject(shortRec);
> }
> }
>
> mvn test -Dtest=CSVRecordTest
>
> [ERROR] testSerialization Time elapsed: 0.032 s <<< ERROR!
> java.io.NotSerializableException: org.apache.commons.csv.CSVParser
> at
> org.apache.commons.csv.CSVRecordTest.testSerialization(CSVRecordTest.java:235)
>
> If I mark the field csvParser as transient it passes. So this is a problem as
> raised by FindBugs.
>
>
>
> I also raised [3] the strange implementation of the CSVParser
> getHeaderNames() which ignores null headers as they cannot be used as a key
> into the map. However the list of column names could contain the null values.
> This test currently fails:
>
> @Test
> public void testHeaderNamesWithNull() throws IOException {
> final Reader in = new StringReader("header1,null,header3\n1,2,3\n4,5,6");
> final Iterator<CSVRecord> records = CSVFormat.DEFAULT.withHeader()
>
> .withNullString("null")
>
> .withAllowMissingColumnNames()
>
> .parse(in).iterator();
> final CSVRecord record = records.next();
> assertEquals(Arrays.asList("header1", null, "header3"),
> record.getParser().getHeaderNames());
> }
>
> I am not saying it should pass but at least the documentation should state
> the behaviour in this edge case. That is the list of header names may be
> shorter than the number of columns when the parser is configured to allow
> null headers. I’ve not raised a bug ticket for this as it is open to opinion
> if this is by design or actually a bug. This issue is still present in 1.8
> RC1.
>
> Previously I suggested documentation changes for this and another edge case
> using the header map to be added to the javadoc for getHeaderNames() and
> getHeaderMap():
>
> - Documentation:
>
> The mapping is only guaranteed to be a one-to-one mapping if the record was
> created with a format that does not allow duplicate or null header names.
> Null headers are excluded from the map and duplicates can only map to 1
> column.
>
>
> - Bug / Documentation
>
> The CSVParser only stores headers names in a list of header names if they are
> not null. So the list can be shorter than the number of columns if you use a
> format that allows empty headers and contains null column names.
>
>
> The ultimate result is that we should document that the purpose of the header
> names is to provide a list of non-null header names in the order they occur
> in the header and thus represent keys that can be used in the header map. In
> certain circumstances there may be more columns in the data than there are
> header names.
>
>
> Alex
>
>
> [1] https://issues.apache.org/jira/browse/CSV-248
> <https://issues.apache.org/jira/browse/CSV-248>
>
> [2]
> https://dist.apache.org/repos/dist/dev/commons/csv/1.8-RC1/site/findbugs.html
> <https://dist.apache.org/repos/dist/dev/commons/csv/1.8-RC1/site/findbugs.html>
>
> [3] https://markmail.org/message/woti2iymecosihx6
> <https://markmail.org/message/woti2iymecosihx6>
>
>
>
> > On 18 Jan 2020, at 17:52, Gary Gregory <ggreg...@apache.org> wrote:
> >
> > We have fixed quite a few bugs and added some significant enhancements
> > since Apache Commons CSV 1.7 was released, so I would like to release
> > Apache Commons CSV 1.8.
> >
> > Apache Commons CSV 1.8 RC1 is available for review here:
> > https://dist.apache.org/repos/dist/dev/commons/csv/1.8-RC1 (svn
> > revision 37670)
> >
> > The Git tag commons-csv-1.8-RC1 commit for this RC is
> > c1c8b32809df295423fc897eae0e8b22bfadfe27 which you can browse here:
> >
> > https://gitbox.apache.org/repos/asf?p=commons-csv.git;a=commit;h=c1c8b32809df295423fc897eae0e8b22bfadfe27
> > You may checkout this tag using:
> > git clone https://gitbox.apache.org/repos/asf/commons-csv.git --branch
> > commons-csv-1.8-RC1 commons-csv-1.8-RC1
> >
> > Maven artifacts are here:
> >
> > https://repository.apache.org/content/repositories/orgapachecommons-1489/org/apache/commons/commons-csv/1.8/
> >
> > These are the artifacts and their hashes:
> >
> > #Release SHA-512s
> > #Sat Jan 18 12:01:01 EST 2020
> > commons-csv-1.8-bin.tar.gz=85a876b41aa9ce61f7f533c46df48754e05bddbdef892aed2bac7674b5ea13855de25576364649048dbb55e7fb18a354305b56cb697e85df68a87113954128ed
> > commons-csv-1.8-bin.zip=9b86a22367c84a0c96a457e8495f81113b64ae5501eabbe2ea4137654b6baa05bcc24a19626452b80e30ff2dd39214840c6ec534be1db9eec2d12c93eeab2de1
> > commons-csv-1.8-javadoc.jar=a481149dfeffe4e915d5d2e846831994223fc7d09ed2b61398c68eed5a672654a141fa6de705aa743d0b5af6fd24a3f4b0d5e7cee238a1f7642673288d4a985d
> > commons-csv-1.8-sources.jar=f68e50f8a025a8b2a570b46905b22b5753a83c19bee5c38103d92ec1e47b4e0d27353e7931961e74fe8e67c4909b0ece6ede49a585d2f9180a7a15458b020bc0
> > commons-csv-1.8-src.tar.gz=c3268f456978e75c19134e35d05bff77002b2fa7439be2623d58a102cab4f93b0913a1a789f962aafcd677938be1547f47c5dd86e3ea08b7bf8f0420e81beb7a
> > commons-csv-1.8-src.zip=ebb32f2406b6afa48472283612c7d0a94f932d7ae7a72ad1d239e2249de12f1e0da7f61d34d95d66b1d1fe95b66b6316af9d1fc93734f610cce4a7163b0900d0
> > commons-csv-1.8-test-sources.jar=13508d417e23a5d3f575a39b3aedb20d0d834335d7994f3045fff316e6b12e50cbf9afe908271357b4091d981c178a28dc61bcdb8db60bd0ada07d3de59eacbf
> > commons-csv-1.8-tests.jar=901889d4be203c2044df89b7e051d21e7b806e5e56438bf9a7483b334331da94b71de1a129c8bf7967e02479a0922bb834ce37eaabf6662702e147813ecb2b7f
> >
> > I have tested this with ' mvn -V -Prelease -Ptest-deploy -P jacoco -P
> > japicmp clean package site deploy' using:
> >
> > Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
> > Maven home: C:\Java\apache-maven-3.6.3\bin\..
> > Java version: 1.8.0_241, vendor: Oracle Corporation, runtime: C:\Program
> > Files\Java\jdk1.8.0_241\jre
> > Default locale: en_US, platform encoding: Cp1252
> > OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"
> >
> > Additional tests with 'mvn -V clean test' using:
> >
> > Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
> > Maven home: C:\Java\apache-maven-3.6.3\bin\..
> > Java version: 1.8.0_241, vendor: Oracle Corporation, runtime: C:\Program
> > Files\Java\jdk1.8.0_241\jre
> > Default locale: en_US, platform encoding: Cp1252
> > OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"
> >
> > Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
> > Maven home: C:\Java\apache-maven-3.6.3\bin\..
> > Java version: 11.0.6, vendor: Oracle Corporation, runtime: C:\Program
> > Files\Java\jdk-11.0.6
> > Default locale: en_US, platform encoding: Cp1252
> > OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"
> >
> > Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
> > Maven home: C:\Java\apache-maven-3.6.3\bin\..
> > Java version: 12.0.2, vendor: Oracle Corporation, runtime: C:\Program
> > Files\Java\jdk-12.0.2
> > Default locale: en_US, platform encoding: Cp1252
> > OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"
> >
> > Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
> > Maven home: C:\Java\apache-maven-3.6.3\bin\..
> > Java version: 13.0.2, vendor: Oracle Corporation, runtime: C:\Program
> > Files\Java\jdk-13.0.2
> > Default locale: en_US, platform encoding: Cp1252
> > OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"
> >
> > Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
> > Maven home: C:\Java\apache-maven-3.6.3\bin\..
> > Java version: 14-ea, vendor: Oracle Corporation, runtime: C:\Program
> > Files\Java\openjdk\jdk-14
> > Default locale: en_US, platform encoding: Cp1252
> > OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"
> >
> > JaCoCo fails on Java 15-EA because it does not know about class file major
> > version 59:
> >
> > Caused by: java.lang.IllegalArgumentException: Unsupported class file major
> > version 59
> > at
> > org.jacoco.agent.rt.internal_43f5073.asm.ClassReader.<init>(ClassReader.java:195)
> >
> > Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
> > Maven home: C:\Java\apache-maven-3.6.3\bin\..
> > Java version: 15-ea, vendor: Oracle Corporation, runtime: C:\Program
> > Files\Java\openjdk\jdk-15
> > Default locale: en_US, platform encoding: Cp1252
> > OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"
> >
> > Details of changes since 1.7 are in the release notes:
> >
> > https://dist.apache.org/repos/dist/dev/commons/csv/1.8-RC1/RELEASE-NOTES.txt
> >
> > https://dist.apache.org/repos/dist/dev/commons/csv/1.8-RC1/site/changes-report.html
> >
> > Site:
> >
> > https://dist.apache.org/repos/dist/dev/commons/csv/1.8-RC1/site/index.html
> > (note some *relative* links are broken and the 1.8 directories are not
> > yet created - these will be OK once the site is deployed.)
> >
> > JApiCmp Report (compared to 1.7):
> >
> > https://dist.apache.org/repos/dist/dev/commons/csv/1.8-RC1/site/japicmp.html
> >
> > RAT Report:
> >
> > https://dist.apache.org/repos/dist/dev/commons/csv/1.8-RC1/site/rat-report.html
> >
> > KEYS:
> > https://www.apache.org/dist/commons/KEYS
> >
> > Please review the release candidate and vote.
> > This vote will close no sooner that 72 hours from now.
> >
> > [ ] +1 Release these artifacts
> > [ ] +0 OK, but...
> > [ ] -0 OK, but really should fix...
> > [ ] -1 I oppose this release because...
> >
> > Thank you,
> >
> > Gary Gregory,
> > Release Manager (using key 86fdc7e2a11262cb)
> >
> > For following is intended as a helper and refresher for reviewers.
> >
> > Validating a release candidate
> > ==============================
> >
> > These guidelines are NOT complete.
> >
> > Requirements: Git, Java, Maven.
> >
> > You can validate a release from a release candidate (RC) tag as follows.
> >
> > 1) Clone and checkout the RC tag
> >
> > git clone https://gitbox.apache.org/repos/asf/commons-csv.git --branch
> > commons-csv-1.8-RC1 commons-csv-1.8-RC1
> > cd commons-csv-1.8-RC1
> >
> > 2) Check Apache licenses
> >
> > This step is not required if the site includes a RAT report page which you
> > then must check.
> >
> > mvn apache-rat:check
> >
> > 3) Check binary compatibility
> >
> > Older components still use Apache Clirr:
> >
> > This step is not required if the site includes a Clirr report page which
> > you then must check.
> >
> > mvn clirr:check
> >
> > Newer components use JApiCmp with the japicmp Maven Profile:
> >
> > This step is not required if the site includes a JApiCmp report page which
> > you then must check.
> >
> > mvn install -DskipTests -P japicmp japicmp:cmp
> >
> > 4) Build the package
> >
> > mvn -V clean package
> >
> > You can record the Maven and Java version produced by -V in your VOTE reply.
> > To gather OS information from a command line:
> > Windows: ver
> > Linux: uname -a
> >
> > 5) Build the site for a single module project
> >
> > Note: Some plugins require the components to be installed instead of
> > packaged.
> >
> > mvn site
> > Check the site reports in:
> > - Windows: target\site\index.html
> > - Linux: target/site/index.html
> >
> > 6) Build the site for a multi-module project
> >
> > mvn site
> > mvn site:stage
> > Check the site reports in:
> > - Windows: target\site\index.html
> > - Linux: target/site/index.html
> >
> > -the end-
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
