Skip to content

[low] fix: chk_dup.py crashes on every run (json.load called on a dict) - #1270

Open
elhoim wants to merge 1 commit into
MISP:mainfrom
elhoim:fix/chk-dup-json-load
Open

elhoim wants to merge 1 commit into
MISP:mainfrom
elhoim:fix/chk-dup-json-load

Conversation

@elhoim

@elhoim elhoim commented Aug 30, 2026

Copy link
Copy Markdown
Member

BLUF — chk_dup.py aborts before it checks anything because json.load is called on a dict.

  • Problemtools/chk_dup.py passes the result of json.load into json.load a second time on the return_paths=False branch, so the tool dies with AttributeError: 'dict' object has no attribute 'read'. Its own __main__ duplicate-name check uses exactly that branch, so the tool has never run.
  • Fix — Load each cluster file once, reuse the result for both branches, and wrap the handles in a context manager with explicit UTF-8 encoding.
  • Effect — Maintainers can actually run chk_dup.py, which now completes and reports 4,355 duplicated names and synonyms already present in the clusters.

Problem

tools/chk_dup.py calls json.load twice on the return_paths=False branch:

if return_paths:
    data.append((filepath, json.load(open(filepath))))
else:
    data.append(json.load(json.load(open(filepath))))   # <-- outer load gets a dict

json.load expects a file object; the outer call is handed the dict the inner call just produced. So running the tool crashes immediately:

  File "/usr/lib/python3.14/json/__init__.py", line 298, in load
    return loads(fp.read(),
                 ^^^^^^^
AttributeError: 'dict' object has no attribute 'read'

chk_dup.py's own duplicate-name detection under if __name__ == '__main__' calls loadjsons("../clusters") with the default return_paths=False, so the tool has never actually run. It has gone unnoticed because the only other caller, tools/chk_empty_strings.py (which CI does exercise via validate_all.sh), passes return_paths=True and hits the working branch.

Fix

Load the file once and reuse the result for both branches. While here, the file handles were never closed on either branch (open() without with) — that is fixed with a context manager, and encoding='utf-8' is set explicitly since 89 of the 131 cluster files contain non-ASCII.

Verification

Both branches now return data:

loadjsons(return_paths=False) -> 131 clusters loaded; first name: Cancer
loadjsons(return_paths=True)  -> 131 tuples; first path: ../clusters/cancer.json

And the tool runs to completion, reporting 4,355 duplicated names/synonyms across galaxies:

Warning duplicate sarcoma
['sarcoma', 'Cancer']
['sarcoma', 'Ransomware']
['sarcoma', 'Malpedia']
Warning duplicate 11
['11', 'NAICS']
['11', 'NACE']
...

This PR only makes the tool run; the duplicates it now reports are cross-galaxy name collisions (many legitimate, e.g. an industry code shared between NAICS and NACE) and are not touched here.

🤖 Generated with Claude Code

loadjsons() called json.load twice on the return_paths=False branch:

    data.append(json.load(json.load(open(filepath))))

The outer call is handed the dict the inner one produced, so the tool dies
with AttributeError: 'dict' object has no attribute 'read'. Since
__main__ calls loadjsons("../clusters") with the default return_paths=False,
the duplicate-name checker has never run. It went unnoticed because
chk_empty_strings.py -- the caller CI exercises -- passes return_paths=True
and hits the working branch.

Load the file once and reuse it. Also close the handles (open() without
`with` on both branches) and set encoding='utf-8', since 89 of the 131
cluster files contain non-ASCII.

The tool now completes and reports 4,355 duplicated names/synonyms.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HZGwPoa8MMfkhCw47rDLA4
@elhoim elhoim changed the title fix: chk_dup.py crashes on every run (json.load called on a dict) [low] fix: chk_dup.py crashes on every run (json.load called on a dict) Sep 3, 2026
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.

1 participant