[$500] Infinite requests when message image source is an invalid link

[m-natarajan]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: Reproducible in staging?: needs reproduction Reproducible in production?: needs reproduction If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: @quinthar Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1707250100494229

Action Performed:

1. Go to any chat
2. Send a message
3. Open react dev tools, components
4. Select the report action item message component for that chat
5. manually edit the action prop, message.html, and set it to this string <h1>GIF Test 1</h1><br /><img src=\"a%20href=\" alt=\"a href=\" />

Expected Result:

No looping requests

Actual Result:

The browser makes requests in a loop

Workaround:

ignore

Platforms:

Which of our officially supported platforms is this issue occurring on?

All

• [ ] Android: Native
• [ ] Android: mWeb Chrome
• [ ] iOS: Native
• [ ] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/38435837/6488bf05-6ab1-4e70-83b0-fae1087b39c6

https://github.com/Expensify/App/assets/38435837/fd2bdc7b-3f84-46c5-91ca-ecff3dabadd1

https://github.com/Expensify/App/assets/38435837/fae755a8-4327-4c0d-ab7a-ac2ad1399464

Notes

For added context, the original problem comes from a message sent by Concierge, where the content comes from the internal concierge compose tool. Somehow that tool send a message with an invalid image source. We have no idea how that happened, but it was during testing, and the tool generally seems to work fine now. Regardless, we don't want NewDot making infinite requests to invalid URLs at all, so let's fix that on the frontend.

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01ef9040e3f909dd5c
• Upwork Job ID: 1755362860038602752
• Last Price Increase: 2024-02-07

[melvin-bot[bot]]

Triggered auto assignment to @dylanexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[neil-marcellini]

I've been investigating in the Slack thread

[neil-marcellini]

@roryabraham do you have any ideas about this, since you implemented the concierge compose updates?

[neil-marcellini]

We found that this only happens for certain messages sent using the Concierge compose tool. The problematic message is the following. If you manually edit a report action message to contain this under the html key, requests will loop infinitely. <h1>GIF Test 1</h1><br /><img src=\"a%20href=\" alt=\"a href=\" />

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~01ef9040e3f909dd5c

[melvin-bot[bot]]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @akinwale (External)

[aswin-s]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Infinite requests when message image source is an invalid link

What is the root cause of that problem?

The HTML render for image uses ThumbnailImage component to display images in report actions.

https://github.com/Expensify/App/blob/4cb6e991e1b95e0cb5c018c0cbf299775c93d491/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js#L83

ThumbnailImage in turn uses ImageWithSizeCalculation component to render the image. The onLoadStart and onLoadEnd props used for Image component are inline functions whose reference changes with each re-render.

https://github.com/Expensify/App/blob/4cb6e991e1b95e0cb5c018c0cbf299775c93d491/src/components/ImageWithSizeCalculation.tsx#L79-L88

This causes the image component to re-render which triggers another network request.

What changes do you think we should make in order to solve the problem?

Memoize onLoadStart & onLoadEnd methods in ImageWithSizeCalculation component.

const onLoadStart = useMemo(() => {
        if (isLoadedRef.current ?? isLoading) {
            return;
        }
        setIsLoading(true);
}, [isLoading])

const onLoadEnd = useMemo(() => {
        setIsLoading(false);
        setIsImageCached(true);
}, [])


return (
        <View style={[styles.w100, styles.h100, style]}>
            <Image
                style={[styles.w100, styles.h100]}
                source={{ uri: url }}
                isAuthTokenRequired={isAuthTokenRequired}
                resizeMode={RESIZE_MODES.cover}
                onLoadStart={onLoadStart}
                onLoadEnd={onLoadEnd}
                onError={onError}
                onLoad={imageLoadedSuccessfully}
            />
            {isLoading && !isImageCached && <FullscreenLoadingIndicator style={[styles.opacity1, styles.bgTransparent]} />}
        </View>
    );

Also onLoadStart method could be further optimised by removing its dependency on state variable isLoading by modifying it like below

const onLoadStart = useMemo(() => {
    setIsLoading(loading => (isLoadedRef.current ?? loading) ? loading : true );
}, [])

What alternative solutions did you explore? (Optional)

None

[paultsimura]

This is a dupe – being fixed in https://github.com/Expensify/App/issues/34601

[neil-marcellini]

Ah great thanks so much @paultsimura . I'm going to HOLD this issue on that one, since this one is a bit different I think it would be wise to re-test it after that issue has been fixed.

[paultsimura]

@neil-marcellini @quinthar is there a way to use the Concierge Compose tool and force it to send a broken image? I'm trying to test the fix to this issue on native platforms, and they don't support the source modification.

[quinthar]

Hm, not really but you could just directly modify a message in Onyx using the Chrome tools I imagine to intentionally break a message.

[paultsimura]

Hm, not really but you could just directly modify a message in Onyx using the Chrome tools I imagine to intentionally break a message.

Yes, I did that, but it doesn't work on iOS and Android apps. So if there's no way to reproduce on these platforms, we might just have to assume that if the web fix works, the native will work as well...

[neil-marcellini]

@paultsimura good point, but I think you can use Flipper dev tools to modify the props of components, and set an invalid message that way.

[dylanexpensify]

held still?

[paultsimura]

held still?

The #34601 is in production, so I think we could take this one off hold and retest.

[roryabraham]

requested retest: https://expensify.slack.com/archives/C9YU7BX5M/p1708648765218909

[neil-marcellini]

You can only really test on dev like this, but it's enough to verify that it's fixed!

https://github.com/Expensify/App/assets/26260477/7289872c-4e48-419f-b172-f1c71b5730a2