Repository: ArchiveBox/ArchiveBox (language: python)

Issue:
Bug: `JSONDecodeError` when trying to load JSON with an array at the top-level
With ArchiveBox version 0.6.2, I used to import URLs stored in JSON files with content looking like the following:
```json
[
    {"url": "https://archivebox.io/", "tags": "test,archivebox"},
    {"url": "https://en.wikipedia.org/wiki/Linux", "tags": "test,wikipedia"}
]
```
```bash
archivebox add --parser json < ./links.json
```
Everything worked well.

With version 0.7.2, however, I have a `JSONDecodeError` exception during the import:
```
  File "/tmp/archivebox/venv/lib/python3.11/site-packages/archivebox/main.py", line 631, in add
    new_links += parse_links_from_source(write_ahead_log, root_url=None, parser=parser)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/archivebox/venv/lib/python3.11/site-packages/archivebox/util.py", line 116, in typechecked_function
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/archivebox/venv/lib/python3.11/site-packages/archivebox/index/__init__.py", line 278, in parse_links_from_source
    raw_links, parser_name = parse_links(source_path, root_url=root_url, parser=parser)
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/archivebox/venv/lib/python3.11/site-packages/archivebox/util.py", line 116, in typechecked_function
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/archivebox/venv/lib/python3.11/site-packages/archivebox/parsers/__init__.py", line 103, in parse_links
    links, parser = run_parser_functions(file, timer, root_url=root_url, parser=parser)
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/archivebox/venv/lib/python3.11/site-packages/archivebox/parsers/__init__.py", line 117, in run_parser_functions
    parsed_links = list(parser_func(to_parse, root_url=root_url))
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/archivebox/venv/lib/python3.11/site-packages/archivebox/parsers/generic_json.py", line 23, in parse_generic_json_export
    links = json.loads(json_file_json_str)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/json/__init__.py", line 346, in loads
    return _default_decoder.decode(s)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/json/decoder.py", line 340, in decode
    raise JSONDecodeError("Extra data", s, end)
json.decoder.JSONDecodeError: Extra data: line 1 column 61 (char 60)
```
The error is caused by the following line, introduced by commit aaca74f: https://github.com/ArchiveBox/ArchiveBox/blob/3ad32509e985236f82f3558f31b856623b1eb261/archivebox/parsers/generic_json.py#L22




Discussion / hints:
ah interesting, all the files I tested with had an object at the top level instead of a list

I can add handling for lists pretty easily, there are so many different JSON formats to support haha
Instead of trying to figure out what is going on when the first line of the JSON file is garbage, it would be easier to try not skipping it first, and then try again after skipping it if that fails.

```python
    try:
        links = json.load(json_file)
    except json.decoder.JSONDecodeError:
        # sometimes the first line is a comment or other junk, so try without
        json_file.seek(0)
        first_line = json_file.readline()
        #print('      > Trying JSON parser without first line: "', first_line.strip(), '"', sep= '')
        links = json.load(json_file)
        # we may fail again, which means we really don't know what to do
```

But maybe even this isn't necessary? It looks like the original "skip the first line" logic came about because ArchiveBox would add the filename to the file as the first line when putting in the sources directory, but that doesn't seem to happen any more (which seems like a much, much better way to go).

Fix patch:
diff --git a/archivebox/parsers/generic_json.py b/archivebox/parsers/generic_json.py
index daebb7c473e..9d12a4ef5f0 100644
--- a/archivebox/parsers/generic_json.py
+++ b/archivebox/parsers/generic_json.py
@@ -18,9 +18,16 @@ def parse_generic_json_export(json_file: IO[str], **_kwargs) -> Iterable[Link]:
 
     json_file.seek(0)
 
-    # sometimes the first line is a comment or filepath, so we get everything after the first {
-    json_file_json_str = '{' + json_file.read().split('{', 1)[-1]
-    links = json.loads(json_file_json_str)
+    try:
+        links = json.load(json_file)
+    except json.decoder.JSONDecodeError:
+        # sometimes the first line is a comment or other junk, so try without
+        json_file.seek(0)
+        first_line = json_file.readline()
+        #print('      > Trying JSON parser without first line: "', first_line.strip(), '"', sep= '')
+        links = json.load(json_file)
+        # we may fail again, which means we really don't know what to do
+
     json_date = lambda s: datetime.strptime(s, '%Y-%m-%dT%H:%M:%S%z')
 
     for link in links:
@@ -59,11 +66,20 @@ def parse_generic_json_export(json_file: IO[str], **_kwargs) -> Iterable[Link]:
             elif link.get('name'):
                 title = link['name'].strip()
 
+            # if we have a list, join it with commas
+            tags = link.get('tags')
+            if type(tags) == list:
+                tags = ','.join(tags)
+            elif type(tags) == str:
+                # if there's no comma, assume it was space-separated
+                if ',' not in tags:
+                    tags = tags.replace(' ', ',')
+
             yield Link(
                 url=htmldecode(url),
                 timestamp=ts_str,
                 title=htmldecode(title) or None,
-                tags=htmldecode(link.get('tags')) or '',
+                tags=htmldecode(tags),
                 sources=[json_file.name],
             )
 


Test patch:
diff --git a/tests/mock_server/templates/example.json b/tests/mock_server/templates/example.json
new file mode 100644
index 00000000000..6ee15597848
--- /dev/null
+++ b/tests/mock_server/templates/example.json
@@ -0,0 +1,6 @@
+[
+{"href":"http://127.0.0.1:8080/static/example.com.html","description":"Example","extended":"","meta":"18a973f09c9cc0608c116967b64e0419","hash":"910293f019c2f4bb1a749fb937ba58e3","time":"2014-06-14T15:51:42Z","shared":"no","toread":"no","tags":"Tag1 Tag2","trap":"http://www.example.com/should-not-exist"},
+{"href":"http://127.0.0.1:8080/static/iana.org.html","description":"Example 2","extended":"","meta":"18a973f09c9cc0608c116967b64e0419","hash":"910293f019c2f4bb1a749fb937ba58e3","time":"2014-06-14T15:51:43Z","shared":"no","toread":"no","tags":"Tag3,Tag4 with Space"},
+{"href":"http://127.0.0.1:8080/static/shift_jis.html","description":"Example 2","extended":"","meta":"18a973f09c9cc0608c116967b64e0419","hash":"910293f019c2f4bb1a749fb937ba58e3","time":"2014-06-14T15:51:44Z","shared":"no","toread":"no","tags":["Tag5","Tag6 with Space"]},
+{"href":"http://127.0.0.1:8080/static/title_og_with_html","description":"Example 2","extended":"","meta":"18a973f09c9cc0608c116967b64e0419","hash":"910293f019c2f4bb1a749fb937ba58e3","time":"2014-06-14T15:51:45Z","shared":"no","toread":"no"}
+]
diff --git a/tests/mock_server/templates/example.json.bad b/tests/mock_server/templates/example.json.bad
new file mode 100644
index 00000000000..88d77757ddf
--- /dev/null
+++ b/tests/mock_server/templates/example.json.bad
@@ -0,0 +1,2 @@
+this line would cause problems but --parser=json will actually skip it
+[{"href":"http://127.0.0.1:8080/static/example.com.html","description":"Example","extended":"","meta":"18a973f09c9cc0608c116967b64e0419","hash":"910293f019c2f4bb1a749fb937ba58e3","time":"2014-06-14T15:51:42Z","shared":"no","toread":"no","tags":"Tag1 Tag2","trap":"http://www.example.com/should-not-exist"}]
diff --git a/tests/test_add.py b/tests/test_add.py
index 331178fe05b..dd1307bbfd4 100644
--- a/tests/test_add.py
+++ b/tests/test_add.py
@@ -91,3 +91,60 @@ def test_extract_input_uses_only_passed_extractors(tmp_path, process):
 
     assert (archived_item_path / "warc").exists()
     assert not (archived_item_path / "singlefile.html").exists()
+
+def test_json(tmp_path, process, disable_extractors_dict):
+    with open('../../mock_server/templates/example.json', 'r', encoding='utf-8') as f:
+        arg_process = subprocess.run(
+            ["archivebox", "add", "--index-only", "--parser=json"],
+            stdin=f,
+            capture_output=True,
+            env=disable_extractors_dict,
+        )
+
+    conn = sqlite3.connect("index.sqlite3")
+    c = conn.cursor()
+    urls = c.execute("SELECT url from core_snapshot").fetchall()
+    tags = c.execute("SELECT name from core_tag").fetchall()
+    conn.commit()
+    conn.close()
+
+    urls = list(map(lambda x: x[0], urls))
+    assert "http://127.0.0.1:8080/static/example.com.html" in urls
+    assert "http://127.0.0.1:8080/static/iana.org.html" in urls
+    assert "http://127.0.0.1:8080/static/shift_jis.html" in urls
+    assert "http://127.0.0.1:8080/static/title_og_with_html" in urls
+    # if the following URL appears, we must have fallen back to another parser
+    assert not "http://www.example.com/should-not-exist" in urls
+
+    tags = list(map(lambda x: x[0], tags))
+    assert "Tag1" in tags
+    assert "Tag2" in tags
+    assert "Tag3" in tags
+    assert "Tag4 with Space" in tags
+    assert "Tag5" in tags
+    assert "Tag6 with Space" in tags
+
+def test_json_with_leading_garbage(tmp_path, process, disable_extractors_dict):
+    with open('../../mock_server/templates/example.json.bad', 'r', encoding='utf-8') as f:
+        arg_process = subprocess.run(
+            ["archivebox", "add", "--index-only", "--parser=json"],
+            stdin=f,
+            capture_output=True,
+            env=disable_extractors_dict,
+        )
+
+    conn = sqlite3.connect("index.sqlite3")
+    c = conn.cursor()
+    urls = c.execute("SELECT url from core_snapshot").fetchall()
+    tags = c.execute("SELECT name from core_tag").fetchall()
+    conn.commit()
+    conn.close()
+
+    urls = list(map(lambda x: x[0], urls))
+    assert "http://127.0.0.1:8080/static/example.com.html" in urls
+    # if the following URL appears, we must have fallen back to another parser
+    assert not "http://www.example.com/should-not-exist" in urls
+
+    tags = list(map(lambda x: x[0], tags))
+    assert "Tag1" in tags
+    assert "Tag2" in tags