summaryrefslogtreecommitdiff
path: root/bugs.org
blob: e1648c8f3afb426b9bc7383b0b793aff8d80584c (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
* Bugs Found in wttrin.el Code Analysis

** BUG [#A] Callback passed to url-retrieve-synchronously (Line 114)
Location: =wttrin.el:114=

The code passes a lambda callback to =url-retrieve-synchronously=:
#+begin_src elisp
(url-retrieve-synchronously
 (concat "https://wttr.in/" query (wttrin-additional-url-params) "A")
 (lambda () (switch-to-buffer (current-buffer))))
#+end_src

The synchronous API signature is =(url-retrieve-synchronously URL &optional silent inhibit-cookies)=. The lambda is being interpreted as the =silent= argument, not a callback. This does nothing but is misleading.

*Fix:* Remove the lambda and just call:
#+begin_src elisp
(url-retrieve-synchronously url)
#+end_src

** BUG [#A] HTTP headers not stripped before xterm-color-filter (Line 152)
Location: =wttrin.el:152=

The code inserts the raw buffer string directly:
#+begin_src elisp
(insert (xterm-color-filter raw-string))
#+end_src

But =raw-string= from =wttrin-fetch-raw-string= includes HTTP headers (status line, Date, Content-Type, etc.). This means the headers are passed through =xterm-color-filter=.

*Fix:* Strip HTTP headers first:
#+begin_src elisp
(goto-char (point-min))
(re-search-forward "\r?\n\r?\n")
(let ((body (buffer-substring-no-properties (point) (point-max))))
  (xterm-color-filter body))
#+end_src

** BUG [#A] No URL encoding of user input (Lines 113, 201)
Location: =wttrin.el:113=, =wttrin.el:201=

User input is concatenated directly into URLs:
#+begin_src elisp
(concat "https://wttr.in/" query (wttrin-additional-url-params) "A")
#+end_src

If a user enters ="New York, NY"= the space and comma could cause URL parsing issues.

*Fix:* URL-encode the location:
#+begin_src elisp
(concat "https://wttr.in/" (url-hexify-string query) ...)
#+end_src

** BUG [#A] No error handling for nil buffer from url-retrieve-synchronously (Line 111)
Location: =wttrin.el:111=

The code assumes =url-retrieve-synchronously= always returns a buffer:
#+begin_src elisp
(with-current-buffer
    (url-retrieve-synchronously ...)
  (decode-coding-string (buffer-string) 'utf-8))
#+end_src

If there's a timeout, DNS failure, or network error, =url-retrieve-synchronously= can return nil, causing an error in =with-current-buffer=.

*Fix:* Check for nil:
#+begin_src elisp
(let ((buf (url-retrieve-synchronously url t t)))
  (if (not buf)
      (error "wttrin: network failure")
    (with-current-buffer buf ...)))
#+end_src

** BUG [#B] Double concatenation of url params (Line 201)
Location: =wttrin.el:201=

In =wttrin--get-cached-or-fetch=:
#+begin_src elisp
(wttrin-fetch-raw-string (concat location (wttrin-additional-url-params)))
#+end_src

But =wttrin-fetch-raw-string= already calls =wttrin-additional-url-params= on line 113. This means the params get added twice.

*Fix:* Remove the concatenation from line 201:
#+begin_src elisp
(wttrin-fetch-raw-string location)
#+end_src

** BUG [#C] wttrin-additional-url-params returns malformed URL (Line 105)
Location: =wttrin.el:105=

The function returns:
#+begin_src elisp
(concat "?" wttrin-unit-system)
#+end_src

If =wttrin-unit-system= is nil, this returns ="?"=. When concatenated with the ="A"= on line 113, you get URLs like =https://wttr.in/Paris?A= instead of =https://wttr.in/Paris?mA= (for metric).

*Fix:* Handle nil properly:
#+begin_src elisp
(defun wttrin-additional-url-params ()
  "Concatenates extra information into the URL."
  (concat "?" (or wttrin-unit-system "")))
#+end_src

Or better yet, restructure the URL building entirely.

** BUG [#C] Hard-coded buffer name prevents multiple locations (Line 139)
Location: =wttrin.el:139=

The buffer is always named ="*wttr.in*"=:
#+begin_src elisp
(let ((buffer (get-buffer-create (format "*wttr.in*")))
#+end_src

This prevents viewing multiple locations simultaneously. The format string has no interpolation.

*Fix:* Use the location name:
#+begin_src elisp
(let ((buffer (get-buffer-create (format "*wttr.in: %s*" location-name)))
#+end_src

** BUG [#C] Fragile header parsing with hard-coded line numbers (Lines 156-165)
Location: =wttrin.el:156-165=

The code assumes weather data always has specific information on lines 4 and 6:
#+begin_src elisp
(forward-line 4)
(setq date-time-stamp ...)
(goto-char (point-min))
(forward-line 6)
(setq location-info ...)
#+end_src

This is fragile and will break if wttr.in changes its output format or if HTTP headers vary in length.

*Fix:* Use pattern matching to find the information instead of relying on fixed line numbers.

** BUG [#C] No buffer cleanup after url-retrieve-synchronously (Line 111)
Location: =wttrin.el:111=

After retrieving the URL, the buffer created by =url-retrieve-synchronously= is never killed. This creates buffer leaks on each weather fetch.

*Fix:* Kill the buffer after extraction:
#+begin_src elisp
(let ((buf (url-retrieve-synchronously url)))
  (when buf
    (unwind-protect
        (with-current-buffer buf
          (decode-coding-string (buffer-string) 'utf-8))
      (kill-buffer buf))))
#+end_src