Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make json.tool soak up input before opening output for writing #89807

Closed
CharString mannequin opened this issue Oct 28, 2021 · 8 comments
Closed

Make json.tool soak up input before opening output for writing #89807

CharString mannequin opened this issue Oct 28, 2021 · 8 comments
Labels
3.9 only security fixes 3.10 only security fixes 3.11 only security fixes type-bug An unexpected behavior, bug, or error

Comments

@CharString
Copy link
Mannequin

CharString mannequin commented Oct 28, 2021

BPO 45644
Nosy @methane, @ambv, @miss-islington, @remilapeyre, @CharString
PRs
  • bpo-45644 Make json.tool soak up infile before writing to outfile #29269
  • bpo-45644: Make json.tool read infile before writing to outfile #29273
  • [3.10] bpo-45644: Make json.tool read infile before writing to outfile (GH-29273) #29445
  • [3.9] bpo-45644: Make json.tool read infile before writing to outfile (GH-29273) #29446
  • bpo-33927: Handle pyton -m json.tool --json-lines infile infile #29478
  • bpo-45644: json.tool: Use staged write for outfile. #30659
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2021-11-06.18:02:21.133>
created_at = <Date 2021-10-28.12:59:07.008>
labels = ['type-bug', '3.9', '3.10', '3.11']
title = 'Make json.tool soak up input before opening output for writing'
updated_at = <Date 2022-01-18.09:34:45.992>
user = 'https://github.com/CharString'

    bugs.python.org fields:

    activity = <Date 2022-01-18.09:34:45.992>
actor = 'methane'
assignee = 'none'
closed = True
closed_date = <Date 2021-11-06.18:02:21.133>
closer = 'lukasz.langa'
components = []
creation = <Date 2021-10-28.12:59:07.008>
creator = 'CharString'
dependencies = []
files = []
hgrepos = []
issue_num = 45644
keywords = ['patch']
message_count = 8.0
messages = ['405182', '405866', '405868', '405872', '405873', '405874', '405913', '405937']
nosy_count = 5.0
nosy_names = ['methane', 'lukasz.langa', 'miss-islington', 'remi.lapeyre', 'CharString']
pr_nums = ['29269', '29273', '29445', '29446', '29478', '30659']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue45644'
versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']
    @CharString
    Copy link
    Mannequin Author

    CharString mannequin commented Oct 28, 2021

    json.tool is very cute and handy for making json readable.

    But rewriting a file in place requires tools like sponge (on POSIX) or a tmpfile, because

    $ python -m json.tool foo.json foo.json

    results in an empty foo.json.

    I propose soaking up the infile before opening the outfile for writing, to prevent that. Much like sort -o does, but without the explicit flag.
    The patch I have prepared changes no behaviours, other than preventing an empty file... (still I see this as an enhancement and not a bug fix)

    @CharString CharString mannequin added type-feature A feature request or enhancement labels Oct 28, 2021
    @ambv
    Copy link
    Contributor

    ambv commented Nov 6, 2021

    This is arguably a bugfix since truncating the infile cannot be construed as a useful feature in any sense. Thus I'm inclined to backport this to 3.10.1 and 3.9.9 too.

    @ambv ambv added 3.9 only security fixes 3.10 only security fixes 3.11 only security fixes type-bug An unexpected behavior, bug, or error and removed type-feature A feature request or enhancement labels Nov 6, 2021
    @ambv
    Copy link
    Contributor

    ambv commented Nov 6, 2021

    New changeset 815dad4 by Chris Wesseling in branch 'main':
    bpo-45644: Make json.tool read infile before writing to outfile (GH-29273)
    815dad4

    @ambv
    Copy link
    Contributor

    ambv commented Nov 6, 2021

    New changeset 6603f6b by Miss Islington (bot) in branch '3.10':
    bpo-45644: Make json.tool read infile before writing to outfile (GH-29273) (GH-29445)
    6603f6b

    @ambv
    Copy link
    Contributor

    ambv commented Nov 6, 2021

    New changeset a932631 by Miss Islington (bot) in branch '3.9':
    bpo-45644: Make json.tool read infile before writing to outfile (GH-29273) (GH-29446)
    a932631

    @ambv
    Copy link
    Contributor

    ambv commented Nov 6, 2021

    Thanks, Chris! ✨ 🍰 ✨

    @ambv ambv closed this as completed Nov 6, 2021
    @ambv ambv closed this as completed Nov 6, 2021
    @remilapeyre
    Copy link
    Mannequin

    remilapeyre mannequin commented Nov 7, 2021

    The proposed path does not work for python -m json.tool --json-lines test.json test.json. There is also #7865 that aims to fix this bug but I did not have the time to get back to it.

    @CharString
    Copy link
    Mannequin Author

    CharString mannequin commented Nov 8, 2021

    @remi

    I left the current behaviour for --json-lines untouched, on purpose.
    My reasoning was that the json-lines format is often seen in JSON streaming, and I didn't want to break the case where the input is an endless stream from stdin or a named pipe. And to keep the patch simple with minimal change to the exposed interface.

    Simplest fix for the case in this current code would be: iff infile equals outfile (minding their types), call list(objs) on the generator[1] to materialise it in one go. The only case where that would break would be when infile == outfile and is a named pipe, but I can't imagine why I would want to both read and write to the same FIFO, other than comedic effect.

    [1]

    cpython/Lib/json/tool.py

    Line 65 in 122ca4d

    objs = (json.loads(line) for line in infile)

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.9 only security fixes 3.10 only security fixes 3.11 only security fixes type-bug An unexpected behavior, bug, or error
    Projects
    JSON issues
    Status: Done
    Milestone
    No milestone
    Development

    No branches or pull requests
    1 participant
    @ambv