fix: conversion html_to_text hr tags rstrip non string object #1162

fabiottini · 2024-07-11T14:19:37Z

Description:

During the conversion from HTML to TEXT ( method html_to_text file: apprise/conversion.py on ´hr´ tag ) if the _result list have a dictionary object inside the method use rstrip on it so raise an Exception:

self._result[-1] = self._result[-1].rstrip(' ')
AttributeError: 'dict' object has no attribute 'rstrip'

Example

self._result = [' ', ' ', {}, 'Hi!', '\n', ' How are you?', '\n', ' ', 'red font', ' ', 'link', ' you wanted. ', {}, ' ', {}]

Checklist

The code change is tested and works locally.
There is no commented out code in this PR.
No lint errors (use flake8)
100% test coverage

caronc · 2024-07-12T00:41:59Z

I'm confused, how can the _result list. Can you share with me an example?

<p>hello world</p>
<hr/>
<caption>Or is it when the hr is broken into 2 items? ... like:</caption>
<hr></hr>

I'm a bit confused on what causes the crash?

fabiottini · 2024-07-12T06:45:29Z

I'm confused, how can the _result list. Can you share with me an example?
hello world
<hr/>
<caption>Or is it when the hr is broken into 2 items? ... like:</caption>
<hr></hr>
I'm a bit confused on what causes the crash?

Hi @caronc,

thanks for the quick support.

During my tests I tried to send through apprise library this simple test message in html format:

<html>
<head></head>
<body>
<p>
   <hr/><b>FROM: </b>apprise-test@xxx.yyy
    <p>
       Hi!<br/>
       How are you?<br/>
       <font color="#FF0000">red font</font> <a href="http://www.python.org">link</a> you wanted.
    </p>
</p>
<br/>
</body>
</html>

and I had this output:

  File "/usr/local/lib/python3.9/site-packages/apprise/plugins/email.py", line 790, in send
    convert_between(
  File "/usr/local/lib/python3.9/site-packages/apprise/conversion.py", line 54, in convert_between
    return convert(content) if convert else content
  File "/usr/local/lib/python3.9/site-packages/apprise/conversion.py", line 80, in html_to_text
    parser.feed(content)
  File "/usr/local/lib/python3.9/html/parser.py", line 110, in feed
    self.goahead(0)
  File "/usr/local/lib/python3.9/html/parser.py", line 170, in goahead
    k = self.parse_starttag(i)
  File "/usr/local/lib/python3.9/html/parser.py", line 342, in parse_starttag
    self.handle_startendtag(tag, attrs)
  File "/usr/local/lib/python3.9/html/parser.py", line 426, in handle_startendtag
    self.handle_starttag(tag, attrs)
  File "/usr/local/lib/python3.9/site-packages/apprise/conversion.py", line 184, in handle_starttag
    self._result[-1] = self._result[-1].rstrip(' ')
AttributeError: 'dict' object has no attribute 'rstrip'

I started to investigate and if I removed hr tags from the message or I fixed the code as suggested in my MR everything works.
In the MR I suggested the code also because it's an additional check to be sure to invoke the method rstrip only on strings.

Info

python: 3.9.18
apprise: 1.8.0

Thanks in advance for the support.
Fabio

caronc · 2024-07-12T11:08:44Z

Perfect, this is what I'm interested in. Can you create a test case for this too with this exact string? You should be able to easily copy/paste examples. I can do this for you too if needed, just give me a few days

…

On Fri, Jul 12, 2024, 2:45 a.m. Fabio Lucattini ***@***.***> wrote: I'm confused, how can the _result list. Can you share with me an example? hello world <hr/> <caption>Or is it when the hr is broken into 2 items? ... like:</caption> <hr></hr> I'm a bit confused on what causes the crash? Hi @caronc <https://github.com/caronc>, thanks for the quick support. During my tests I tried to send through apprise library this simple test message in html format: <html><head></head><body> <hr/>FROM: ***@***.*** Hi! How are you? red font <a href="http://www.python.org">link</a> you wanted. </body></html> and I had this output: File "/usr/local/lib/python3.9/site-packages/apprise/plugins/email.py", line 790, in send convert_between( File "/usr/local/lib/python3.9/site-packages/apprise/conversion.py", line 54, in convert_between return convert(content) if convert else content File "/usr/local/lib/python3.9/site-packages/apprise/conversion.py", line 80, in html_to_text parser.feed(content) File "/usr/local/lib/python3.9/html/parser.py", line 110, in feed self.goahead(0) File "/usr/local/lib/python3.9/html/parser.py", line 170, in goahead k = self.parse_starttag(i) File "/usr/local/lib/python3.9/html/parser.py", line 342, in parse_starttag self.handle_startendtag(tag, attrs) File "/usr/local/lib/python3.9/html/parser.py", line 426, in handle_startendtag self.handle_starttag(tag, attrs) File "/usr/local/lib/python3.9/site-packages/apprise/conversion.py", line 184, in handle_starttag self._result[-1] = self._result[-1].rstrip(' ') AttributeError: 'dict' object has no attribute 'rstrip' I started to investigate and if I removed hr tags from the message or I fixed the code as suggested in my MR everything works. In the MR I suggested the code also because it's an additional check to be sure to invoke the method rstrip only on strings. Info - python: 3.9.18 - apprise: 1.8.0 Thanks in advance for the support. Fabio — Reply to this email directly, view it on GitHub <#1162 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAGPTRTRZ5YP6UU2MO2S67LZL53R7AVCNFSM6AAAAABKXC6DKOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRUHEZDKNZSGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

fabiottini · 2024-07-12T11:59:44Z

Perfect, this is what I'm interested in. Can you create a test case for this too with this exact string? You should be able to easily copy/paste examples. I can do this for you too if needed, just give me a few days
…
On Fri, Jul 12, 2024, 2:45 a.m. Fabio Lucattini @.> wrote: I'm confused, how can the _result list. Can you share with me an example?
hello world

Or is it when the hr is broken into 2 items? ... like:
I'm a bit confused on what causes the crash? Hi @caronc https://github.com/caronc, thanks for the quick support. During my tests I tried to send through apprise library this simple test message in html format:

FROM: @.
Hi!
How are you?
red font link you wanted.

and I had this output: File "/usr/local/lib/python3.9/site-packages/apprise/plugins/email.py", line 790, in send convert_between( File "/usr/local/lib/python3.9/site-packages/apprise/conversion.py", line 54, in convert_between return convert(content) if convert else content File "/usr/local/lib/python3.9/site-packages/apprise/conversion.py", line 80, in html_to_text parser.feed(content) File "/usr/local/lib/python3.9/html/parser.py", line 110, in feed self.goahead(0) File "/usr/local/lib/python3.9/html/parser.py", line 170, in goahead k = self.parse_starttag(i) File "/usr/local/lib/python3.9/html/parser.py", line 342, in parse_starttag self.handle_startendtag(tag, attrs) File "/usr/local/lib/python3.9/html/parser.py", line 426, in handle_startendtag self.handle_starttag(tag, attrs) File "/usr/local/lib/python3.9/site-packages/apprise/conversion.py", line 184, in handle_starttag self._result[-1] = self._result[-1].rstrip(' ') AttributeError: 'dict' object has no attribute 'rstrip' I started to investigate and if I removed hr tags from the message or I fixed the code as suggested in my MR everything works. In the MR I suggested the code also because it's an additional check to be sure to invoke the method rstrip only on strings. Info - python: 3.9.18 - apprise: 1.8.0 Thanks in advance for the support. Fabio — Reply to this email directly, view it on GitHub <#1162 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGPTRTRZ5YP6UU2MO2S67LZL53R7AVCNFSM6AAAAABKXC6DKOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRUHEZDKNZSGQ . You are receiving this because you were mentioned.Message ID: @.***>

Hi @caronc,

sure! I'll prepare the tests in a couple of days and I'll give you back the feedback.

Thanks for your support.
Fabio

after few errors during the library utilization test the correct fix in the code for the HR tag

fabiottini · 2024-07-15T10:28:13Z

Hi @caronc ,

as promise I have done the tests and I checked that also the workflows tests on github runs without problem.
Let me know that it's ok or if I have to do something more to help.

Thanks in advance,
Fabio

fabiottini · 2024-07-15T10:53:50Z

Hi @caronc,

I added a space in a comment during the typing.. now i checked with flake8 test/test_conversion.py and pytest and with github workflows pipeline and seems everything it's ok on my changes.
I notice an error in another test:

Let me know if it's ok or you need support.

Thanks in advance,
Fabio

caronc · 2024-07-16T20:44:49Z

This isn't you, i'm not sure what causes these random test failures to be honest. I'll have to look further into this.

In the meantime, thank you very much for this PR

fabiottini · 2024-07-17T08:46:46Z

This isn't you, i'm not sure what causes these random test failures to be honest. I'll have to look further into this.

In the meantime, thank you very much for this PR

Hi @caronc ,

I did a bit of investigating on the error in the pipeline test to figure out what appened.

It seems the invocation of obj.notify on line 1192 sometimes calls logout randomly.

I changed the code like this to identify the problem:

...
        for no, _ in enumerate(range(notifications), start=1):
            # Clean our slate
            mock_post.reset_mock()
            mock_get.reset_mock()
            mock_put.reset_mock()

            # Before notification
            print("Before notification")
            print(mock_post.call_args_list)

            assert obj.notify(
                body='body', title='title', notify_type=NotifyType.INFO
            ) is True

            # After notification
            print("After notification")
            print(mock_post.call_args_list)

            assert mock_get.call_count == 0
            assert mock_post.call_count == 0

            print(mock_post.call_args_list)  # Debugging output
            
            assert mock_put.call_count == 1
            assert mock_put.call_args_list[0][0][0] == \
                'http://localhost/_matrix/client/v3/rooms/' + \
                f'%21abc123%3Alocalhost/send/m.room.message/{no}'
...

The output was:

Before notification
[]
After notification
[]
[]
Before notification
[]
After notification
[]
[]
Before notification
[]
After notification
[]
[]
Before notification
[]
After notification
[]
[]
Before notification
[]
After notification
[]
[]
Before notification
[]
After notification
[]
[]
Before notification
[]
After notification
[]
[]
Before notification
[]
After notification
[]
[]
Before notification
[]
After notification
[]
[]
Before notification
[]
After notification
[call('https://localhost/_matrix/client/r0/logout', data='{}', params=None, headers={'User-Agent': 'Apprise', 'Content-Type': 'application/json', 'Accept': 'application/json', 'Authorization': 'Bearer abcd1234'}, verify=True, timeout=(4.0, 4.0))]
=================================================================================== short test summary info ===================================================================================
FAILED test/test_plugin_matrix.py::test_plugin_matrix_transaction_ids_api_v3 - AssertionError: assert 1 == 0

caronc · 2024-07-17T11:07:21Z

This is amazing detective work on your part! 🚀🚀

caronc · 2024-07-17T19:43:50Z

You saved me hours of debugging; what you identified so quickly was infinit whileloops i had running on this side unable to hang/crash on this 1 line. But it's nto the first time it's happened, so it really had me sratching my head. I THINK i fixed it in #1166 so i'm going to take the risk that it fixes it for your build too and merge. Worst cast, i'll fix it thereafter.

I really appreciate all of your great work

Edit: It merged smoothly; so we nailed the issue! Thank you (again) for your amazing detective work and figuring out the extra matrix:// web call that was being made.

fix: conversion html_to_text hr tags rstrip non string object

f668314

fabiottini added 7 commits July 15, 2024 10:41

test: html_to_text adding hr test

e2089de

after few errors during the library utilization test the correct fix in the code for the HR tag

chore: flake8 fix style

55e8c29

test: fix output test

7521689

test: fix endline append ---

d4a5a59

test: fixed and add multiple tests on same HR problem

20bac9e

test: fix new line and spaces

f747db6

test: add tests case on HR specifically for dict

381c699

chore: whitespace comment EOL

24b4d54

caronc mentioned this pull request Jul 17, 2024

Fixes random matrix:// unittest failures #1166

Merged

4 tasks

caronc merged commit e30adb4 into caronc:master Jul 17, 2024
6 of 12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: conversion html_to_text hr tags rstrip non string object #1162

fix: conversion html_to_text hr tags rstrip non string object #1162

fabiottini commented Jul 11, 2024 •

edited by caronc

Loading

caronc commented Jul 12, 2024

fabiottini commented Jul 12, 2024

caronc commented Jul 12, 2024 via email

fabiottini commented Jul 12, 2024

fabiottini commented Jul 15, 2024

fabiottini commented Jul 15, 2024

caronc commented Jul 16, 2024

fabiottini commented Jul 17, 2024

caronc commented Jul 17, 2024

caronc commented Jul 17, 2024 •

edited

Loading

fix: conversion html_to_text hr tags rstrip non string object #1162

fix: conversion html_to_text hr tags rstrip non string object #1162

Conversation

fabiottini commented Jul 11, 2024 • edited by caronc Loading

Description:

Example

Checklist

caronc commented Jul 12, 2024

fabiottini commented Jul 12, 2024

Info

caronc commented Jul 12, 2024 via email

fabiottini commented Jul 12, 2024

fabiottini commented Jul 15, 2024

fabiottini commented Jul 15, 2024

caronc commented Jul 16, 2024

fabiottini commented Jul 17, 2024

caronc commented Jul 17, 2024

caronc commented Jul 17, 2024 • edited Loading

fabiottini commented Jul 11, 2024 •

edited by caronc

Loading

caronc commented Jul 17, 2024 •

edited

Loading