internetarchive / internetarchive/heritrix3

ExtractorJson does not extract URIs nested inside JSON arrays

Open Beginner friendly
#768 1 comment 0 reactions 0 assignees View on GitHub
bug pull request welcome
Dominant language
Java
Stars
3.3k
Forks
793
Avg merge
1d 8h
Merged PRs (30d)
8

Description

`ExtractorJson` extracts a URI held in an object property, but not the same URI
nested inside a JSON array. Reproduced on **3.17.1**.

**In a default configuration this has no effect**, since `ExtractorJson` is not in
the default profile's extractor chain — so this is not urgent for anyone's running
crawls. It is reported because the behaviour is silent: an array-shaped JSON
document yields no outlinks and no error, which looks the same as a document that
genuinely has no links in it.

### Reproduction

A drop-in test is below. It goes at
`modules/src/test/java/org/archive/modules/extractor/ExtractorJsonTest.java` and
runs with `mvn test -Dtest=ExtractorJsonTest`.

It was run here against the 3.17.1 distribution jars, on the JUnit Platform
console runner:

```
ExtractorJsonTest
+-- testBareUriStringInsideArrayIsExtracted() [X] expected: <[http://example.org/target.json]> but was: <[]>
+-- testUriInsideArrayIsExtracted() [X] expected: <[http://example.org/target.json]> but was: <[]>
'-- testUriInObjectPropertyIsExtracted() [OK]

[ 1 tests successful ]
[ 2 tests failed ]
```

All three cases carry the same URI and differ only in where it sits:

```json
{"pointer": {"href": "http://example.org/target.json"}} <- extracted
{"pointers": [{"href": "http://example.org/target.json"}]} <- not extracted
{"pointers": ["http://example.org/target.json"]} <- not extracted
```

The object-property case passing is the point of including it: the empty results
are about array handling, not about the fixture or the test setup.

### What appears to cause it

`parse()` handles the array case with:

```java
} else if (field.getValue().isArray()) {
field.getValue()
.propertyStream()
.forEach(fieldValue -> parse(fieldValue.getValue(), links));
}
```

`propertyStream()` is called on an `ArrayNode`. Jackson documents it as returning
properties *"iff this node is an `ObjectNode`. For other types of nodes, returns
empty stream"*, and `ArrayNode` does not override it — so the stream is empty and
the array's elements are never visited. Independently of Heritrix, on the same
`jackson-databind` the distribution bundles:

```
node.isArray() : true
node.propertyStream() : 0 <- what the array branch iterates
node.valueStream() : 1 <- the elements that are there
```

### Versions

- Reproduced on **3.17.1** (`jackson-databind` 2.22.2) and on 3.13.0
(`jackson-databind` 2.20.1), so it is not specific to a Jackson version.
- `ExtractorJson.java` is byte-identical between the `3.13.0` tag and `master`,
so no later release has changed this.

### One question

Was skipping arrays intended when the extractor was added in #701 — for instance
to avoid speculative requests from long record lists — or should array elements be
traversed like object values?

ExtractorJsonTest.java

```java
/*
* This file is part of the Heritrix web crawler (crawler.archive.org).
*
* Licensed to the Internet Archive (IA) by one or more individual
* contributors.
*
* The IA 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.archive.modules.extractor;

import org.archive.modules.CrawlURI;
import org.archive.net.UURIFactory;
import org.junit.jupiter.api.Test;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

import static org.junit.jupiter.api.Assertions.assertEquals;

/**
* Unit test for ExtractorJson.
*
*

The same URI is presented twice: once as a plain object property, once
* nested inside an array. {@link #testUriInObjectPropertyIsExtracted()} passes
* and {@link #testUriInsideArrayIsExtracted()} does not, so the second result is
* about array handling rather than about the fixture or the harness.
*/
public class ExtractorJsonTest {

private static final String URI_IN_DOCUMENT = "http://example.org/target.json";

@Test
public void testUriInObjectPropertyIsExtracted() throws Exception {
assertExtracted("{\"pointer\":{\"href\":\"" + URI_IN_DOCUMENT + "\"}}");
}

@Test
public void testUriInsideArrayIsExtracted() throws Exception {
assertExtracted("{\"pointers\":[{\"href\":\"" + URI_IN_DOCUMENT + "\"}]}");
}

@Test
public void testBareUriStringInsideArrayIsExtracted() throws Exception {
assertExtracted("{\"pointers\":[\"" + URI_IN_DOCUMENT + "\"]}");
}

private void assertExtracted(String json) throws Exception {
CrawlURI curi = new CrawlURI(
UURIFactory.getInstance("http://example.org/document.json"),
null, null, LinkContext.NAVLINK_MISC);
curi.setContentType("application/json");
curi.setRecorder(ContentExtractorTestBase.createRecorder(json, "UTF-8"));
curi.setContentSize(json.length());
curi.setFetchStatus(200);

ExtractorJson extractor = new ExtractorJson();
extractor.setLoggerModule(new UnitTestUriLoggerModule());
extractor.process(curi);

// compared as strings rather than as CrawlURIs so that a failure names
// the URI that was missed
List extracted = new ArrayList();
for (CrawlURI outlink : curi.getOutLinks()) {
extracted.add(outlink.getURI());
}
Collections.sort(extracted);

assertEquals(Collections.singletonList(URI_IN_DOCUMENT), extracted);
}
}
```

Found while looking at extractor behaviour on JSON content.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in modules/src/main/java/org/archive/modules/extractor/ExtractorJson.java, focusing on parse() and its array branch. Review the supplied regression cases in modules/src/test/java/org/archive/modules/extractor/ExtractorJsonTest.java, then run mvn test -Dtest=ExtractorJsonTest. Done means both array cases extract the target URI while the existing object-property case still passes.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
88/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.