54ac / stream-detector

A Firefox addon for keeping track of manifests used by various streaming protocols and downloading media files.
https://addons.mozilla.org/en-US/firefox/addon/hls-stream-detector/
Mozilla Public License 2.0
642 stars 96 forks source link

fix: Check for header and value existance #103

Closed DanSM-5 closed 2 years ago

DanSM-5 commented 2 years ago

Edited: Remove additional changes

Issue: Using the extension I got an error Cannot read properties of undefined (reading 'value') and it is because the second evaluation for e happens regardless of the existence of header.

        if (header)
            // go through content types and see if the header matches
            e =
                customCtPref === true &&
                customSupported.ct?.includes(header.value.toLowerCase()) &&
                customSupported;
               // here will run regardless of the existence of header
        if (!e) 
            e = supported.find((f) => f.ct.includes(header.value.toLowerCase()));

Proposed fix: Evaluate header and value first as both expressions relay on them.

        if (header && header.value) {
            // go through content types and see if the header matches
            e =
                customCtPref === true &&
                customSupported.ct?.includes(header.value.toLowerCase()) &&
                                customSupported;
            if (!e)
                e = supported.find((f) => f.ct.includes(header.value.toLowerCase()));
        }

Disregard this part Additional changes: I noticed that customSupported is evaluated at the end in two expressions. If customSupported.ct and customSupported.ext are present, customSupported will evaluate to true for sure but probably the intention is to identify it first before accessing the properties, so I moved the evaluation first in line 83 and 95.

DanSM-5 commented 2 years ago

This error is not critical but it will get rid of the error message in the extensions tab

54ac commented 2 years ago

Thanks for the pull request.

I think a more streamlined approach would be to simply use customSupported?.ct?.includes(header.value.toLowerCase()) to evaluate it step by step, but that works too.

customSupported is not evaluated at the end but assigned to e, so it must remain there - equivalent to something like e = customCtPref && customSupported?.ct?.includes(header.value.toLowerCase()) ? customSupported : null.

DanSM-5 commented 2 years ago

Sorry, I totally missed that part. I reverted that change on customSupported

54ac commented 2 years ago

Cool, thanks.