Uploaded image for project: 'Flink'
  1. Flink
  2. FLINK-5561

DataInputDeserializer#available returns one too few

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.2.0
    • Fix Version/s: 1.2.0, 1.3.0
    • Component/s: None
    • Labels:
      None

      Description

      DataInputDeserializer#available seems to assume that the position points to the last read byte but instead it points to the next byte. Therefore, it returns a value which is 1 smaller than the correct one.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user NicoK opened a pull request:

          https://github.com/apache/flink/pull/3171

          FLINK-5561 fix DataInputDeserializer#available() 1 smaller than correct

          This also adds a unit test for `DataInputDeserializer#available()` - the first one for `DataInputDeserializer` unfortunately, but adding more is a different issue.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/NicoK/flink flink-5561

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/flink/pull/3171.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #3171


          commit e29d5ef3099d5f5ffc817f3e5cdbb79c523b34c7
          Author: Nico Kruber <nico@data-artisans.com>
          Date: 2017-01-18T17:52:57Z

          FLINK-5561 fix DataInputDeserializer#available() 1 smaller than correct


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user NicoK opened a pull request: https://github.com/apache/flink/pull/3171 FLINK-5561 fix DataInputDeserializer#available() 1 smaller than correct This also adds a unit test for `DataInputDeserializer#available()` - the first one for `DataInputDeserializer` unfortunately, but adding more is a different issue. You can merge this pull request into a Git repository by running: $ git pull https://github.com/NicoK/flink flink-5561 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/3171.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #3171 commit e29d5ef3099d5f5ffc817f3e5cdbb79c523b34c7 Author: Nico Kruber <nico@data-artisans.com> Date: 2017-01-18T17:52:57Z FLINK-5561 fix DataInputDeserializer#available() 1 smaller than correct
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user uce commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3171#discussion_r97060478

          — Diff: flink-runtime/src/test/java/org/apache/flink/runtime/util/DataInputDeserializerTest.java —
          @@ -0,0 +1,58 @@
          +/*
          + * Licensed to the Apache Software Foundation (ASF) under one
          + * or more contributor license agreements. See the NOTICE file
          + * distributed with this work for additional information
          + * regarding copyright ownership. The ASF licenses this file
          + * to you under the Apache License, Version 2.0 (the
          + * "License"); you may not use this file except in compliance
          + * with the License. You may obtain a copy of the License at
          + *
          + * http://www.apache.org/licenses/LICENSE-2.0
          + *
          + * Unless required by applicable law or agreed to in writing, software
          + * distributed under the License is distributed on an "AS IS" BASIS,
          + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
          + * See the License for the specific language governing permissions and
          + * limitations under the License.
          + */
          +
          +package org.apache.flink.runtime.util;
          +
          +import org.junit.Assert;
          +import org.junit.Test;
          +
          +import java.io.IOException;
          +
          +/**
          + * Test suite for the

          {@link DataInputDeserializer}

          class.
          + */
          +public class DataInputDeserializerTest {
          +
          + @Test
          + public void testAvailable() throws Exception {
          + byte[] bytes;
          + DataInputDeserializer dis;
          +
          + bytes = new byte[] {};
          + dis = new DataInputDeserializer(bytes, 0, bytes.length);
          + Assert.assertEquals(bytes.length, dis.available());
          +
          + bytes = new byte[]

          {1, 2, 3}

          ;
          + dis = new DataInputDeserializer(bytes, 0, bytes.length);
          + Assert.assertEquals(bytes.length, dis.available());
          +
          + dis.readByte();
          + Assert.assertEquals(2, dis.available());
          + dis.readByte();
          + Assert.assertEquals(1, dis.available());
          + dis.readByte();
          + Assert.assertEquals(0, dis.available());
          +
          + try

          { + dis.readByte(); + }

          catch (IOException e) {
          + // ignore
          — End diff –

          If you are testing for an expected Excepteion here, you should add a `fail` after the link you expect to throw the Exception:

          ```java
          try

          { dis.readBytes() // expected to throw Assert.fail("Did not throw expected Exception"); }

          catch (IOException ignored) {
          }
          ```

          Show
          githubbot ASF GitHub Bot added a comment - Github user uce commented on a diff in the pull request: https://github.com/apache/flink/pull/3171#discussion_r97060478 — Diff: flink-runtime/src/test/java/org/apache/flink/runtime/util/DataInputDeserializerTest.java — @@ -0,0 +1,58 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.runtime.util; + +import org.junit.Assert; +import org.junit.Test; + +import java.io.IOException; + +/** + * Test suite for the {@link DataInputDeserializer} class. + */ +public class DataInputDeserializerTest { + + @Test + public void testAvailable() throws Exception { + byte[] bytes; + DataInputDeserializer dis; + + bytes = new byte[] {}; + dis = new DataInputDeserializer(bytes, 0, bytes.length); + Assert.assertEquals(bytes.length, dis.available()); + + bytes = new byte[] {1, 2, 3} ; + dis = new DataInputDeserializer(bytes, 0, bytes.length); + Assert.assertEquals(bytes.length, dis.available()); + + dis.readByte(); + Assert.assertEquals(2, dis.available()); + dis.readByte(); + Assert.assertEquals(1, dis.available()); + dis.readByte(); + Assert.assertEquals(0, dis.available()); + + try { + dis.readByte(); + } catch (IOException e) { + // ignore — End diff – If you are testing for an expected Excepteion here, you should add a `fail` after the link you expect to throw the Exception: ```java try { dis.readBytes() // expected to throw Assert.fail("Did not throw expected Exception"); } catch (IOException ignored) { } ```
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user NicoK commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3171#discussion_r97063511

          — Diff: flink-runtime/src/test/java/org/apache/flink/runtime/util/DataInputDeserializerTest.java —
          @@ -0,0 +1,58 @@
          +/*
          + * Licensed to the Apache Software Foundation (ASF) under one
          + * or more contributor license agreements. See the NOTICE file
          + * distributed with this work for additional information
          + * regarding copyright ownership. The ASF licenses this file
          + * to you under the Apache License, Version 2.0 (the
          + * "License"); you may not use this file except in compliance
          + * with the License. You may obtain a copy of the License at
          + *
          + * http://www.apache.org/licenses/LICENSE-2.0
          + *
          + * Unless required by applicable law or agreed to in writing, software
          + * distributed under the License is distributed on an "AS IS" BASIS,
          + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
          + * See the License for the specific language governing permissions and
          + * limitations under the License.
          + */
          +
          +package org.apache.flink.runtime.util;
          +
          +import org.junit.Assert;
          +import org.junit.Test;
          +
          +import java.io.IOException;
          +
          +/**
          + * Test suite for the

          {@link DataInputDeserializer}

          class.
          + */
          +public class DataInputDeserializerTest {
          +
          + @Test
          + public void testAvailable() throws Exception {
          + byte[] bytes;
          + DataInputDeserializer dis;
          +
          + bytes = new byte[] {};
          + dis = new DataInputDeserializer(bytes, 0, bytes.length);
          + Assert.assertEquals(bytes.length, dis.available());
          +
          + bytes = new byte[]

          {1, 2, 3}

          ;
          + dis = new DataInputDeserializer(bytes, 0, bytes.length);
          + Assert.assertEquals(bytes.length, dis.available());
          +
          + dis.readByte();
          + Assert.assertEquals(2, dis.available());
          + dis.readByte();
          + Assert.assertEquals(1, dis.available());
          + dis.readByte();
          + Assert.assertEquals(0, dis.available());
          +
          + try

          { + dis.readByte(); + }

          catch (IOException e) {
          + // ignore
          — End diff –

          sure, that makes sense

          Show
          githubbot ASF GitHub Bot added a comment - Github user NicoK commented on a diff in the pull request: https://github.com/apache/flink/pull/3171#discussion_r97063511 — Diff: flink-runtime/src/test/java/org/apache/flink/runtime/util/DataInputDeserializerTest.java — @@ -0,0 +1,58 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.runtime.util; + +import org.junit.Assert; +import org.junit.Test; + +import java.io.IOException; + +/** + * Test suite for the {@link DataInputDeserializer} class. + */ +public class DataInputDeserializerTest { + + @Test + public void testAvailable() throws Exception { + byte[] bytes; + DataInputDeserializer dis; + + bytes = new byte[] {}; + dis = new DataInputDeserializer(bytes, 0, bytes.length); + Assert.assertEquals(bytes.length, dis.available()); + + bytes = new byte[] {1, 2, 3} ; + dis = new DataInputDeserializer(bytes, 0, bytes.length); + Assert.assertEquals(bytes.length, dis.available()); + + dis.readByte(); + Assert.assertEquals(2, dis.available()); + dis.readByte(); + Assert.assertEquals(1, dis.available()); + dis.readByte(); + Assert.assertEquals(0, dis.available()); + + try { + dis.readByte(); + } catch (IOException e) { + // ignore — End diff – sure, that makes sense
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/flink/pull/3171

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/3171
          Hide
          uce Ufuk Celebi added a comment -

          Fixed in 108865d (release-1.2), 6c4644d (master).

          Show
          uce Ufuk Celebi added a comment - Fixed in 108865d (release-1.2), 6c4644d (master).

            People

            • Assignee:
              NicoK Nico Kruber
              Reporter:
              NicoK Nico Kruber
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development