On Sun, Jan 19, 2020 at 7:55 PM sebb <seb...@gmail.com> wrote:
> 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.
>
I think the larger context is whether we can effectively remove (or leave
broken) serialization outside of a major version.
Gary
>
> 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
>
>
