Consider svg images and (web)fonts to be resources.

simplesamlphp / SAML-tracer

Browser extension for examining SAML messages

https://addons.mozilla.org/nl/firefox/addon/saml-tracer/

BSD 2-Clause "Simplified" License

142 stars 39 forks source link

Consider svg images and (web)fonts to be resources. #34

Closed thijskh closed 6 years ago

thijskh commented 6 years ago

The 'filter resources' option now also filters svg images and webfonts.

khlr commented 6 years ago

Nice, Thijs! These requests can be really annoying. Could you additionally add application/font-woff? I noticed this type, too.

Moreover there can be often a bunch of application/json-requests that bloat the trace window. But I think this type may be a bit critically since these requests could be a (desired) response of an web API or something like that. What do you think?

thijskh commented 6 years ago

Added application/font-woff. I think filtering out json is out of scope at least for this option, that tries to filter out things that we are 'sure' to be irrelevant.

khlr commented 6 years ago

Agreed!

khlr commented 6 years ago

Just one other small thing: Please additionally add application/font-woff2.

tvdijen commented 6 years ago

The amount of types to pass is probably smaller than the amount of excludes... Wouldn't it be better to define the passes, and exclude anything else?

khlr commented 6 years ago

I agree that it's very likely that there are more ex- than includes, Tim. But I think Thijs mentioned a very important point: If it's not really certain that a request can be discarded it'd be safer to have it included.

I could very well imagine that'd be fairly annoying if relevant requests would be filtered away due to an application/* content-type.

On the other hand it'd be hard to safely predict all relevant content-types. Hence I think it'd be a good idea stick with the current exclude-approach and exclude only those request that are safe to exclude.

tvdijen commented 6 years ago

In that case, we should exclude audio/. video/ www/ and message/ ... IMHO, it's not the end of the world if something legit gets excluded... We can always show it with the button.

thijskh commented 6 years ago

I've added application/font-woff2.

@tvdijen My approach to this issue is quite pragmatic:

The code already works with a switch/case statement, so adding some more types is a simple change and does not require additional code changes;
I don't see that many video/* stuff polluting my SAML traces in practice (as in: I never see that). So this simple change makes it work just fine for me. I'm leaving my PR at this for now.