Skip to content

Fix encoding on locales with non-arabic digits (as Hindi (India))#3

Merged
dampcake merged 2 commits intodampcake:masterfrom
EonTechnology:master
Jul 11, 2018
Merged

Fix encoding on locales with non-arabic digits (as Hindi (India))#3
dampcake merged 2 commits intodampcake:masterfrom
EonTechnology:master

Conversation

@EonTechnology
Copy link
Contributor

Unit tests for Bencode on different locales (if needed):

package com.dampcake.bencode;

import static org.junit.Assert.assertEquals;

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.concurrent.ConcurrentSkipListMap;

import org.junit.AfterClass;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

/**
 * Unit tests for Bencode on different locales.
 */
@SuppressWarnings("unchecked")
@RunWith(Parameterized.class)
public class BencodeLocaleTest {

    private static Locale startLocale = Locale.getDefault();

    @Rule
    public ExpectedException exception = ExpectedException.none();

    @Parameterized.Parameter
    public Locale testLocale;

    private Bencode instance;

    @AfterClass
    public static void restore() throws Exception {
        Locale.setDefault(startLocale);
    }

    @Parameterized.Parameters(name = "{index}: {0}")
    public static Collection<Object[]> data() {
        LinkedList<Object[]> res = new LinkedList<Object[]>();
        for (Locale locale : Locale.getAvailableLocales()) {
            res.add(new Object[] {locale});
        }
        return res;
    }

    @Before
    public void setUp() {
        Locale.setDefault(testLocale);
        instance = new Bencode();
    }

    @Test
    public void testDecodeDictionary() throws Exception {
        Map<String, Object> decoded = instance.decode(
                "d4:dictd3:1234:test3:4565:thinge4:listl11:list-item-111:list-item-2e6:numberi123456e6:string5:valuee".getBytes(),
                Type.DICTIONARY);

        assertEquals(4, decoded.size());

        assertEquals("value", decoded.get("string"));
        assertEquals(123456L, decoded.get("number"));

        List<Object> list = (List<Object>) decoded.get("list");
        assertEquals(2, list.size());
        assertEquals("list-item-1", list.get(0));
        assertEquals("list-item-2", list.get(1));

        Map<String, Object> map = (Map<String, Object>) decoded.get("dict");
        assertEquals(2, map.size());
        assertEquals("test", map.get("123"));
        assertEquals("thing", map.get("456"));
    }

    @Test
    public void testWriteDictionary() throws Exception {
        byte[] encoded = instance.encode(new HashMap<Object, Object>() {{
            put("string", "value");
            put("number", 123456);
            put("list", new ArrayList<Object>() {{
                add("list-item-1");
                add("list-item-2");
            }});
            put("dict", new ConcurrentSkipListMap() {{
                put(123, "test");
                put(456, "thing");
            }});
        }});

        assertEquals(
                "d4:dictd3:1234:test3:4565:thinge4:listl11:list-item-111:list-item-2e6:numberi123456e6:string5:valuee",
                new String(encoded, instance.getCharset()));
    }

    @Test
    public void testDecodeEncodedDictionary() throws Exception {
        byte[] encoded = instance.encode(new HashMap<Object, Object>() {{
            put("string", "value");
            put("number", 123456);
            put("list", new ArrayList<Object>() {{
                add("list-item-1");
                add("list-item-2");
            }});
            put("dict", new ConcurrentSkipListMap() {{
                put(123, "test");
                put(456, "thing");
            }});
        }});

        Map<String, Object> decoded = instance.decode(encoded, Type.DICTIONARY);

        assertEquals(4, decoded.size());

        assertEquals("value", decoded.get("string"));
        assertEquals(123456L, decoded.get("number"));

        List<Object> list = (List<Object>) decoded.get("list");
        assertEquals(2, list.size());
        assertEquals("list-item-1", list.get(0));
        assertEquals("list-item-2", list.get(1));

        Map<String, Object> map = (Map<String, Object>) decoded.get("dict");
        assertEquals(2, map.size());
        assertEquals("test", map.get("123"));
        assertEquals("thing", map.get("456"));
    }
}

if (s == null) throw new NullPointerException("s cannot be null");

return String.format("%d%s%s", s.length(), Bencode.SEPARATOR, s);
return String.format("%s%s%s", Integer.toString(s.length()), Bencode.SEPARATOR, s);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this do? When would s.legnth() ever return anything that would need to be toString'd?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

String.format("%d", ...) uses locale digits.
Integer.toString(...) uses "normal" digits.
Or locale may be set on method calls: String.format(Locale.ENGLISH, "%d", ...).

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. I think I would rather use the Locale.ENGLISH solution as that looks to be the more accepted solution. Also if you could update the BencodeOutputStreamTest to run parameterized with all the locales that would be awesome!

…tStreamTest (tests on all available locales).
@dampcake dampcake merged commit 5e5eb22 into dampcake:master Jul 11, 2018
@dampcake
Copy link
Owner

I was hoping to get this released to maven today. Looks like it will take a little while longer (troubleshooting with sonatype right now).

For now you can use the SNAPSHOT if you need it right away.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants