Fix for SvgUri and SvgCssUri crashing on non-existing svg files

[SjaakSchilperoort]

Summary

Fix for interpreting an HTML error response as svg data, and then crash when this cannot be parsed as well formed XML

closes #1500

Test Plan

n/a

What's required for testing (prerequisites)?

Just a a clean project with the latest versions of react-native and react-native-svg

What are the steps to reproduce (after prerequisites)?

Include this in App.js

<SvgUri width={100} height={100} uri={'https://dev.w3.org/SVG/tools/svgweb/samples/svg-files/ruby.svg'} />

and you'll get:

Then make the file name invalid

<SvgUri width={100} height={100} uri={'https://dev.w3.org/SVG/tools/svgweb/samples/svg-files/xruby.svg'} />

and you'll get:

With the fix you'll get this instead:

Compatibility

OS	Implemented
iOS	✅
Android	✅

Checklist

• I have tested this on a device and a simulator
• I added documentation in README.md
• I updated the typed files (typescript)
• I added a test for the API in the __tests__ folder

[jorgemasta]

Good catch! I think is a good improvement

[Ivanacc]

It would be great to add this fix! :)

[tomsnep]

I also still have this problem and is quite a critical one if you are not fully in charge of the svg code. Any updates on this one?

[SjaakSchilperoort]

@tomsnep If this fix is critical for you, you can add patch-package to your project, save the patch below as patches/react-native-svg+12.1.1.patch and you're good to go.

diff --git a/node_modules/react-native-svg/src/xml.tsx b/node_modules/react-native-svg/src/xml.tsx
index 828f104..a8fea26 100644
--- a/node_modules/react-native-svg/src/xml.tsx
+++ b/node_modules/react-native-svg/src/xml.tsx
@@ -124,7 +124,7 @@ export function SvgXml(props: XmlProps) {
 
 export async function fetchText(uri: string) {
   const response = await fetch(uri);
-  return await response.text();
+  return response.ok ? await response.text() : null
 }
 
 export function SvgUri(props: UriProps) {

[zrina1314]

why hasn't this library been updated for a long time

[WoLewicki]

I double-checked the code I think it would be even better to throw Error instead of console.warn there, since this method is used with catch in SvgUri, so the Error is caught and can be managed by the user in onError prop of SvgUri: https://github.com/react-native-svg/react-native-svg/blob/650275fdf5f0e3ff3fc51e8b2d8b98fd34110d1c/src/xml.tsx#L137. So such an example would work:

import React, {Component} from 'react';
import {  SvgUri } from 'react-native-svg';

class Example extends Component {

    state={
      uri: 'https://dev.w3.org/SVG/tools/svgweb/samples/svg-files/xruby.svg',
    }

  render() {
    return (
      <>
        <SvgUri width={100} onError={err => this.setState({uri: 'https://dev.w3.org/SVG/tools/svgweb/samples/svg-files/ruby.svg'})} height={100} uri={this.state.uri} />
      </>
    );
  }
}

What do you think of it?

[WoLewicki]

I will also change the target branch since now main is the default branch to merge commits to.

[SjaakSchilperoort]

Yes, looks good.

Note that component SvgCssUri, which I use in my code, doesn't support property onError. I guess that adding support for this is a different issue.

[WoLewicki]

Yeah, I think it was missed in this commit: 3c32a6f. Could you make a PR with such change for SvgCssUri? I also checked where else fetchText is used and it looks like there is another place where it could be handled with onError: https://github.com/react-native-svg/react-native-svg/blob/54e40251a491bb1a2d0a75e4748a23ea24fb3f6b/src/LocalSvg.tsx#L58 Could you add same .catch(err) with an option of onError there too? It would be handled everywhere then 🎉 As for this PR, I think it is ready now so I'll merge it 🎉 Thanks for your contribution.

[SjaakSchilperoort]

Yeah, I think it was missed in this commit: 3c32a6f. Could you make a PR with such change for SvgCssUri?

See PR #1718